Supposedly moveq should be allowed in an it block according to this:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0802b/Cjabicci.html

Here are some other instances where this is reported as a warning when the instructions are supposed to be permitted:

../../third_party/skia/include/private/SkFixed.h:113:14: warning: deprecated instruction in IT block [-Winline-asm]
            "rsbcs   %1, %1, #0             \n"
             ^
<inline asm>:8:1: note: instantiated into assembly here
rsbcs   r9, r9, #0             

=========
../../third_party/skia/src/opts/SkBlitRow_opts_arm_neon.cpp:207:65: warning: deprecated instruction in IT block [-Winline-asm]
                      "it eq                                  \n\t"
                                                                ^
<inline asm>:6:2: note: instantiated into assembly here
        moveq      ip, #8              

There are also a lot of errors under 'third_party/ffmpeg/', but some of them are legitimately deprecated.

Opened upstream llvm bug https://llvm.org/bugs/show_bug.cgi?id=31885

For the warning reported for moveq, According to Amaury from Arm, compiler ends up generating mvneq r6, 21 which is a 32 bit instruction as moveq with -22 immediate cannot be encoded in 16 bit mode.

Use of 32 bit instructions is deprecated in IT block.

I confirm that:
rsbcs  r9, r9, #0
moveq  ip, #8
moveq  r6,#-22 (mvneq r6, 21)

are all using the 32bit variant of the instruction encoding.

I'll see tomorrow if I can find a workaround for at least:
third_party/lss/linux_syscall_support.h
hoping it will only affect breakpad which is used exceptionally and has very targeted unittests

Assigning to Adenislon Cavalcanti.

Guys,

need your help.
I have a partial fix for Chromium that needs to be checked on ChromeOS. But I don't know how to make the chromeos build system to switch to using LLVM.
As you know the Linux build uses LLVM while ChromeOS is still using gcc.
Any help would be appreciated.

Since the fix is in third party libraries, I'll open bugs against Skia and ffmpeg and upstream the patches as soon as possible, regardless of the clang build on ChromeOS as long as the Linux build does not cough up any errors building chrome. Is that the right approach?

Thanks

Hi Amaury,
I am not sure how are you building chrome for chromeOS, but if you are using build_packages script, this should work:

./setup_board --board=${BOARD} --profile=llvm --force
USE="clang" ./build_packages --board=${BOARD} ....rest of args...

Hi Manoj,

Thanks for the tip. 
Is there a public answer for the time frame of the switch?
The reason I'm asking is because I see errors popping up in the build of Chrome for ChromeOS I've never seen before that I don't see building Chrome for Linux and wonder if I should fix the errors reported by gcc as well.

It could be because the build switched to a new release of gcc (or simply a new revision), or it could be because they use different code path, all true, but it'd be nice to know if the llvm switch will become reality at some point, and if the build with llvm completes successfully I can simply forget about the issues reported by gcc.

Hi Amaury,

We plan to move chrome to clang soon, hopefully 2-3 weeks from now. Therefore, It is ok to ignore gcc errors for now. 
Btw, can you share what errors are you getting from the gcc build so that we can take a look?

Another bug report for the warnings in linux_syscall_support.h:
https://bugs.chromium.org/p/linux-syscall-support/issues/detail?id=18

Amaury, can you let us know the current status of the warnings? We would like to pull in the upstream patches.

linux_syscall_support.h is ready for upload
ffmpeg: Waiting for authorization to push my patch
skia: still checking the validity and the performance impact

I'll check the new bug ASAP.

I've noticed that the IT warning has become a silent warning. Is there a way to get ninja to treat it as an error?

Can you remove these lines from cros_chrome_sdk.py (chromite/cli/cros/cros_chrome_sdk.py) . It should be under chrome checkout.

      #  crbug.com/686903 
620:      clang_append_flags = ['-Wno-inline-asm']  - Remove this line

Pushed patch to lss

https://chromium-review.googlesource.com/c/446525/

Just to be sure, are you guys waiting for me about the lss patch?

I pinged Mike (vapier@) for lss patch review. Can you please let us know the status for other projects?

The following revision refers to this bug:
  https://chromium.googlesource.com/linux-syscall-support/+/c555f533313986d29c827eb59c7bd99fd37b3ec4

commit c555f533313986d29c827eb59c7bd99fd37b3ec4
Author: Amaury Le Leyzour <amaury.leleyzour@arm.com>
Date: Wed Mar 01 20:59:27 2017

Fix armv8/thumb IT blocks

ARMv8 specifies that an IT block should be followed by only one
16-bit instruction.
Moved the conditional code outside the inline assembly to let the
compiler make the right choice.

BUG= 686903 

Change-Id: I1b218dab30e395ea707dc08b82a26a3e3d2a54e4
Reviewed-on: https://chromium-review.googlesource.com/446525
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Mark Seaborn <mseaborn@chromium.org>

[modify] https://crrev.com/c555f533313986d29c827eb59c7bd99fd37b3ec4/linux_syscall_support.h

Manoj,

Skia, under internal review. Should be done before eow. Includes a performance improvement and a simple but really dirty fix.
ffmpeg, still waiting for authorization to upstream
lss for arm (as opposed to thumb) is under review as well but is of lesser priority.
Do you if any new issues have appeared lately? I'll check with a clean build tonight

Sent a patch to Skia:

https://skia-review.googlesource.com/c/9340/

ffmpeg is still pending

unfortunately, the LSS change is broken in some cases.  under CrOS, we're seeing:

armv7a-cros-linux-gnueabi-g++ -DHAVE_CONFIG_H -I. -I... -I./src  -I.../src   -Wmissing-braces -Wnon-virtual-dtor -Woverloaded-virtual -Wreorder -Wsign-compare -Wunused-variable -Wvla -fPIC -O2 -O2 -O2 -pipe -march=armv7-a -mfpu=vfpv3-d16 -mfloat-abi=hard -g -std=c++11 -c -o src/client/linux/handler/exception_handler.o .../src/client/linux/handler/exception_handler.cc
.../src/client/linux/handler/exception_handler.cc: In member function ‘bool google_breakpad::ExceptionHandler::GenerateDump(google_breakpad::ExceptionHandler::CrashContext*)’:
.../src/client/linux/handler/exception_handler.cc:554:1: error: r7 cannot be used in asm here
 }
 ^
.../src/client/linux/handler/exception_handler.cc:554:1: error: r7 cannot be used in asm here

where are these failures showing up?

when trying to use the new code in the CrOS CQ.  changes landed in breakpad don't automatically show up in CrOS -- we have to sync them.

https://uberchromegw.corp.google.com/i/chromeos/builders/whirlwind-paladin/builds/7256/steps/BuildPackages/logs/stdio

it looks clean now. Was it reverted?

We can't access the reported URL. In worst case I guess is OK to revert to
clear the bots.

I've tried building a fresh new CrOS with gcc for veyron_jerry, and I have the same error:
<third_party/lss> % git checkout 5cedb6bf4e42ebb0a90603535321a265b72d3709
<third_party/lss> % git diff master

diff --git a/linux_syscall_support.h b/linux_syscall_support.h
index f74eda1..a5bcb9b 100644
--- a/linux_syscall_support.h
+++ b/linux_syscall_support.h
@@ -2594,19 +2594,27 @@ struct kernel_statfs {
                                    int flags, void *arg, int *parent_tidptr,
                                    void *newtls, int *child_tidptr) {
       long __res;
-      if (fn == NULL || child_stack == NULL) {
-        __res = -EINVAL;
-      } else {
+      {
         register int   __flags __asm__("r0") = flags;
         register void *__stack __asm__("r1") = child_stack;
         register void *__ptid  __asm__("r2") = parent_tidptr;
         register void *__tls   __asm__("r3") = newtls;
         register int  *__ctid  __asm__("r4") = child_tidptr;
-        __asm__ __volatile__(/* Push "arg" and "fn" onto the stack that will be
+        __asm__ __volatile__(/* if (fn == NULL || child_stack == NULL)
+                              *   return -EINVAL;
+                              */
+                             "cmp   %2,#0\n"
+                             "it    ne\n"
+                             "cmpne %3,#0\n"
+                             "it    eq\n"
+                             "moveq %0,%1\n"
+                             "beq   1f\n"
+
+                             /* Push "arg" and "fn" onto the stack that will be
                               * used by the child.
                               */
-                             "str   %4,[%2,#-4]!\n"
-                             "str   %1,[%2,#-4]!\n"
+                             "str   %5,[%3,#-4]!\n"
+                             "str   %2,[%3,#-4]!\n"
 
                              /* %r0 = syscall(%r0 = flags,
                               *               %r1 = child_stack,
@@ -2614,7 +2622,7 @@ struct kernel_statfs {
                               *               %r3 = newtls,
                               *               %r4 = child_tidptr)
                               */
-                             "mov r7, %8\n"
+                             "mov r7, %9\n"
                              "swi 0x0\n"
 
                              /* if (%r0 != 0)
@@ -2647,11 +2655,12 @@ struct kernel_statfs {
 
                              /* Call _exit(%r0).
                               */
-                             "mov r7, %9\n"
+                             "mov r7, %10\n"
                              "swi 0x0\n"
                            "1:\n"
                              : "=r" (__res)
-                             : "r"(fn), "r"(__stack), "r"(__flags), "r"(arg),
+                             : "i"(-EINVAL),
+                               "r"(fn), "r"(__stack), "r"(__flags), "r"(arg),
                                "r"(__ptid), "r"(__tls), "r"(__ctid),
                                "i"(__NR_clone), "i"(__NR_exit)
                              : "cc", "r7", "lr", "memory");

<third_party/lss> % ~/trunk/src/scripts/build_packages --board=veyron_jerry


chromeos-chrome-58.0.3011.0_rc-r1: ../../../../../../../home/$USER/chrome_root/src/breakpad/src/client/linux/handler/exception_handler.cc:554:1: error: r7 cannot be used in asm here


That's one of the issues I was referencing to when I talked about CrOS not building anymore using gcc.
Should I wait for clang to become the compiler of choice to push this patch? 
Do you guys plan to keep the gcc build working whatever?

Let me know if the patch revert works for you, and what could be the next step.

Thanks

I'm working on a patch to remove the use of r7 in clone.
Looks like it's building with gcc, but still need to check the clang build. Let me know if that's something you'd like to have

while CrOS might be moving to clang only, LSS as a standalone project must support both clang & gcc, so we will need a real fix to work on both compilers :/

Hmmm, looks like CrOS builds everything using clang by default now.
Could someone please tell me how to build the whole thing using gcc?

Amaury, can you try this: 
USE="-clang" ./build_packages ...

BTW, I've pushed a new version of the Skia patch: 
https://skia-review.googlesource.com/c/9346/

First one was rejected. Sorry about that

The r7 issue is purealy a gcc issue. clang is not affected by it.
The reason is because r7 is used to hold the frame pointer, and gcc forbids inline assembly to use it.

I think the whole problem is not on the patch itself, but on the default ISA that gcc and clang choose to use.
Remember that gcc uses thumb by default, while clang uses arm by default. Or at least, the upstream compilers behave like this. 

I've found this in breakpad/BUILD.gn:

    if (current_cpu == "arm" && is_chromeos) {
      # Avoid running out of registers in
      # linux_syscall_support.h:sys_clone()'s inline assembly.
      cflags = [ "-marm" ]
    }

While I do see the -marm on the gcc command line, it's overloaded by a later -mthumb. The use of -marm does in fact solve the issue.
Could it be that the switch to clang required -mthumb to appear on the command line which makes that workaround invalid if gcc is the compiler of choice?

Another option could be to change the gcc options this way instead:

    if (current_cpu == "arm" && is_chromeos && !is_clang) {
      cflags = [ "-fomit-frame-pointer" ]
    }

gcc doesn't have an opinion on thumb-vs-arm -- that's left up to the person configuring it to decide.  in CrOS, we enable thumb by default.

can't we figure out a way to work with both ?  on x86/PIC, it's semi-common to deal with gcc's ebx restrictions by munging the registers behind gcc's back.  either by marking an otherwise unused register as clobber and doing a swap, or manually pushing/popping it inside the asm block.

My understanding of the issue is that if you want to build this code with gcc and use thumb and use the frame pointer, r7 must be protected, and gcc will complain if you do. So there are ways to fix this issue in all the cases and it means reverting this change in third_party/lss:
https://crrev.com/1549d20f6d3e7d66bb4e687c0ab9da42c2bff2ac

Which relates to this bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1247399

and 
https://codereview.chromium.org/1903833002

Which according to the logs fixed an issue on the mozilla side, and was probably (I can  only speculate here) hidden on the chromium side because of the arm compile mode that does not have the r7 restriction defined in the ABI. 

Regarding breakpad's pre-existing, but apparently non-functional work-around here:
https://cs.chromium.org/chromium/src/breakpad/BUILD.gn?q=marm&l=636
(from: https://crrev.com/2db8b425c8da36a5d3b2b23e1a7753a399b32a11 just five days before the original r7-push/pop workaround)

I'm not sure when the workaround stopped working (ie., at what point -mthumb was added to the end of the GCC command line), but to bring it back to life I think it should be possible to follow this example:

https://cs.chromium.org/chromium/src/base/allocator/BUILD.gn?q=compiler_arm_thumb&l=227
(from: https://crrev.com/c0e12589bf7ae45f0f463d4726224442af90e142 )

This might work to fix any build configurations that are still affected by this r7 problem.

However, if there are no such builds then it seems like the right answer is to ignore it because it's an old compiler bug that shouldn't manifest for anybody using modern toolchains.  Does that sound right?

keep in mind that breakpad is used in many places.  the BUILD.gn applies when breakpad is built directly into the Chromium browser.  it doesn't apply to any other build setup.  Chromium OS for example builds breakpad standalone for its tools and so it can link into other things.  we noticed the r7 error in that scenario.

i don't think forcing -marm everywhere makes sense or is correct.  nor is it an old compiler bug -- gcc-4.9 & gcc-5.3 fail in the same way when producing thumb code.

if gcc can't handle this scenario, then let's drop r7 from the clobber list and take care of saving/restoring it in the inline assembly ourselves.

Dropping r7 from the clobber list and putting it on the stack was done here:
https://crrev.com/77ebebeffe70f6c41493f239e0a1bc449cc6b009
and reverted here:
https://crrev.com/1549d20f6d3e7d66bb4e687c0ab9da42c2bff2ac

It was reverted because when you don't include r7 in the clobber list, GCC can decide to use it as one of the argument registers, resulting in faulty code.

It might not make a lot of sense for GCC to insert r7 in the same context as where it protests that it's reserved, but apparently it can.  One instance where this makes sense is when the compiler flags include -fomit-frame-pointer.  Another might be when the code or optimisations are perturbed and it decides that a stack frame is unnecessary for this function.

I suppose it's possible to cuddle the push and pop tightly around the SWI, but I think that operation returns twice, so it has to be pushed on both stacks.

i think if we push r7 before "mov r7, %8", and then pop it at the "1:" label, we should be OK.  the child will mess around with r7 & its new stack anyways before it calls the target function, so the current stack doesn't matter to it.  and it will call _exit before the "1:" label.  so only the parent will execute code after that point.

I have uploaded a safer version of linux_syscall_support.h to the lss project that removes the problem of gcc vs clang. The issue of the compiler assigning r7 to a register has also been taken care of by moving the result into __res after the clone has happened should fix the issue reported and fixed by commit 1549d20f6d3e7d66bb4e687c0ab9da42c2bff2ac.

Here's the link:
https://chromium-review.googlesource.com/c/466548/

Hi Amaury, Can you let us know of the current status?
Thanks!

If I understood correctly, his patch (from April 3th) wasn't reviewed?

Amaury is at vacations now, should be back at office in a few days.

I have uploaded this atch for lss:
https://chromium-review.googlesource.com/c/466548/

And this one for Skia (March 8):
https://skia-review.googlesource.com/c/9346/

Still have a few legal hurdles regarding the ffmpeg patch.

Did I forget something?

Thanks Amaury,
I have pinged the reviewers. Please let me know if there is no response within a few days.

Hi Manoj,

I've pinged the LSS project maintainer, but no news since the begining of May.
On the ffmpeg front, I finally have a green light to push the patch for review. 
Just a few more details to finalize it, and we should be good.

The following revision refers to this bug:
  https://chromium.googlesource.com/linux-syscall-support/+/a91633d172407f6c83dd69af11510b37afebb7f9

commit a91633d172407f6c83dd69af11510b37afebb7f9
Author: Amaury Le Leyzour <amaury.leleyzour@arm.com>
Date: Fri Jun 16 03:49:05 2017

Remove "r7 cannot be used in asm" gcc error

By using push pop and removing r7 for the list of constraints,
we can workaround gcc and its protection of r7 as r7 holds
the frame pointer.

BUG= 686903 

Change-Id: I6f3d6dbe8d36ae615c9cef33bd906dbf6b9bf527
Reviewed-on: https://chromium-review.googlesource.com/466548
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a91633d172407f6c83dd69af11510b37afebb7f9/linux_syscall_support.h

The following revision refers to this bug:
  https://chromium.googlesource.com/breakpad/breakpad/+/c142362a6c1dc5c67bb56a8e46c19199831991bc

commit c142362a6c1dc5c67bb56a8e46c19199831991bc
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Jun 16 04:05:08 2017

roll lss deps

This should hopefully fix the ARMv8/IT fix to work on clang & gcc.

BUG= chromium:686903 

Change-Id: Ib99f05a0cd8df2cb2d393e2ff951d3109cdb5f5b
Reviewed-on: https://chromium-review.googlesource.com/538213
Reviewed-by: Mark Mentovai <mark@chromium.org>

[modify] https://crrev.com/c142362a6c1dc5c67bb56a8e46c19199831991bc/default.xml
[modify] https://crrev.com/c142362a6c1dc5c67bb56a8e46c19199831991bc/DEPS

Hi Amaury, is ffmpeg the only remaining issue here?

i've rolled lss & breakpad in CrOS now and they seem to be sticking everywhere, so that's all set from my pov.  thanks!

I have not checked any other builds of ARMv8 with the flags to treat those warnings as errors. So it could have changed since the last time I've checked.

There is one that pops up in my mind that could be tricky: boringssl. Tricky because I don't ARM is allowed to do anything there. I can check with my management and our lawyers.

Anyway, there is also a discussion as whether this should be treated as an error since the status has evolved from deprecated to "performance deprecated". Even if performance deprecated is not really defined.
I know you guys have talked to our LLVM team about it (Kristof Beyls in particular), and the latest I've heard is that gnu binutils and LLVM probably will get a better answer

Let me know what you want to do with this. 

[It appears that a bunch of old cros issues bulk-added the "Infra" component recently, but they should probably be "Infra>Client>ChromeOS".]

Hi Amaury, is there an update you can share regarding ffmpeg?

Amaury has left ARM and no one else in the team is working on this issue.

I guess we can mark it as 'wont fix'.