[PATCH] D112811: [ARM] implement LOAD_STACK_GUARD for remaining targets

Mon Nov 29 17:43:52 PST 2021

nickdesaulniers added a comment.

In D112811#3159051 <https://reviews.llvm.org/D112811#3159051>, @hans wrote:

> In D112811#3144510 <https://reviews.llvm.org/D112811#3144510>, @hans wrote:
>
>> We bisected some Chromium test failures on Android, which points to this change (https://crbug.com/1270361). We'll investigate further, just wanted to give a heads up.
>
> What we're hitting seems to be that the LOAD_STACK_GUARD pseudo-instruction can be register allocated to one of the high Thumb registers (in our case r12), but gets expanded using the tLDRLIT_ga_pcrel pseudo, which cannot take a high Thumb register as output.
>
> It tries anyway, and generates the following textual asm:
>
>   ldr     r12, .LCPI0_0 
>
> but what it actually encodes is this:
>
>   4: 11 4c         ldr     r4, [pc, #68]
>
> (Note the different register.)
>
> I've uploaded a reproducer here: https://bugs.chromium.org/p/chromium/issues/detail?id=1270361#c30

Seems like LLVM is not choosing the wide encoding, whereas GAS is:

  $ arm-linux-gnueabi-as foo.s
  $ arm-linux-gnueabi-objdump -dr a.out
  ...
     4:   f8df c038       ldr.w   ip, [pc, #56]   ; 40 <_ZN2v88internal4wasm16LiftoffAssembler13emit_i32_addiENS0_8RegisterES3_i+0x40>
  ...

  $ llc -filetype=obj -relocation-model=pic -o a.out a.ll
  $ arm-linux-gnueabi-objdump -dr a.out
  ...
     4:   4c0d            ldr     r4, [pc, #52]   ; (3c <_ZN2v88internal4wasm16LiftoffAssembler13emit_i32_addiENS0_8RegisterES3_i+0x3c>)
  ...

I don't see anything in `F5.1.73 LDR (literal)` of the ARM ARM regarding the use of of `ip`/`r12`.  GAS even errors if I try to force the narrow encoding with a `.n` suffix:

  foo.s:33: Error: cannot honor width suffix -- `ldr.n r12,.LCPI0_0'

It looks like GAS doesn't support a dest/`Rt` greater than `r7` for the narrow encoding?  Do we not have a way to emit the wide encoding?

> In any case, unless you can think of an immediate fix, I'd like to revert this in the meantime.

This patch has a child patch as well; D112768 <https://reviews.llvm.org/D112768>.

Follow up: https://reviews.llvm.org/D107872.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112811/new/

https://reviews.llvm.org/D112811