%rip was only included for 64-bit RIP-relative relocations, but needs to be included for 32-bit as well.
Sure, added. I had also wrongly written "PIC-relative" which should clearly be "RIP-relative", edited that as well.
I wonder whether a fastisel-* test file is more suitable.
I had started with a new test, but switched to extending pic.ll instead because it was more thorough. I can copy pic.ll to fastisel-pic.ll and only check x32 fastisel in there, if preferred.
I don't. If you're willing to commit it, I'd appreciate it, author info is "Harald van Dijk <email@example.com>". Otherwise I'm pretty sure I can pester someone about it after the weekend :)
@hvdijk On EXPENSIVE_CHECKS builds I'm seeing a build failure in pic.ll - please can you take a look?
Command Output (stdout): -- $ ":" "RUN: at line 1" $ "e:\llvm\ninja\bin\llc.exe" "-mcpu=generic" "-mtriple=i686-pc-linux-gnu" "-relocation-model=pic" "-asm-verbose=false" "-post-RA-scheduler=false" $ "e:\llvm\ninja\bin\filecheck.exe" "E:\llvm\llvm-project\llvm\test\CodeGen\X86\pic.ll" "-check-prefixes=CHECK,CHECK-I686" $ ":" "RUN: at line 2" $ "e:\llvm\ninja\bin\llc.exe" "-mcpu=generic" "-mtriple=x86_64-pc-linux-gnux32" "-relocation-model=pic" "-asm-verbose=false" "-post-RA-scheduler=false" # command stderr: # After X86 Fixup SetCC # Machine code for function test6: IsSSA, TracksLiveness Constant Pool: cp#0: [double 1.234560e+02, double 4.561230e+02], align=8 Function Live Ins: $edi in %0 bb.0.entry: liveins: $edi %0:gr32 = COPY $edi %5:gr32 = MOV32r0 implicit-def $eflags TEST32rr %0:gr32, %0:gr32, implicit-def $eflags %1:gr8 = SETCCr 4, implicit $eflags %6:gr32 = INSERT_SUBREG %5:gr32(tied-def 0), %1:gr8, %subreg.sub_8bit %3:gr32 = LEA64_32r $rip, 1, $noreg, %const.0, $noreg %4:fr64 = MOVSDrm_alt killed %3:gr32, 8, killed %6:gr32, 0, $noreg :: (load 8 from constant-pool) $xmm0 = COPY %4:fr64 RET 0, $xmm0 # End machine code for function test6. *** Bad machine code: Illegal virtual register for instruction *** - function: test6 - basic block: %bb.0 entry (0x26ed631e9a0) - instruction: %4:fr64 = MOVSDrm_alt killed %3:gr32, 8, killed %6:gr32, 0, $noreg :: (load 8 from constant-pool) - operand 3: killed %6:gr32 Expected a GR32_NOSP register, but got a GR32 register LLVM ERROR: Found 1 machine code errors.
Sorry about that, already doing so. The code change in here is correct, I think, but there are two independent pre-existing problems that are exposed by the test when expensive checks are enabled. For one I have opened https://reviews.llvm.org/D91924, I am still looking for the right way to fix the other one.
What is the status of the expensive check failure? It has been present for five days now. Should we revert this patch until that is resolved?
(However, I don't know if we are more permissive with failures in the expensive checks jobs than other jobs.)
Should we revert this patch until that is resolved?
The test could be disabled for x32 until those other fixes go in though, if wanted. There should be no need to revert the functional part of this patch: the breakage is unrelated to it, this is merely something that was previously not covered by any tests.