Page MenuHomePhabricator

[ARM] additionally check for ARM::INLINEASM_BR w/ ARM::INLINEASM
ClosedPublic

Authored by nickdesaulniers on May 24 2019, 9:01 AM.

Details

Summary

We were observing failures for arm32 allyesconfigs of the Linux kernel
with the asm goto Clang patch, where ldr's were being generated to
offsets too far away to encode in imm12.

It looks like since INLINEASM_BR was created off of INLINEASM, a few
checks for INLINEASM needed to be updated to check for either case.

pr/41999

Link: https://github.com/ClangBuiltLinux/linux/issues/490

Diff Detail

Repository
rL LLVM

Event Timeline

peter.smith accepted this revision.May 24 2019, 9:28 AM

Code change looks good to me. All the places that ARM::INLINE_ASM are mentioned now take into account ARM::INLINEASM_BR.

Would it be possible to try and write a test case that fails without it? I'm thinking of something like:

extern int other(int *v);

void func(void) {
    int val = 0x12345678; // Generate load from constant pool entry
    other(&val);
}

In theory if there are enough asm goto before the end of the function (constant pool goes at the end by default) then this should trigger the problem. I wouldn't want to hold up the overall asm-goto effort just for the sake of a test case though so I've marked as approved.

This revision is now accepted and ready to land.May 24 2019, 9:28 AM

constant pool goes at the end by default

looks like the function is getting split in two, with the constant pool going in the middle.

func:
  @ function prolog
  ldr     r0, .LCPI0_1
  @ do stuff w/ r0
  b       .LBB0_2
.LCPI0_1:
        .long   305419896               @ 0x12345678
        .p2align        2
.LBB0_2:
        @APP
        @ asm goto nop sled I created
This comment was removed by nickdesaulniers.
This comment was removed by nickdesaulniers.

Landing for now to unblock asm goto https://reviews.llvm.org/D56571. I will try to get a test in for this later.

This revision was automatically updated to reflect the committed changes.