[PATCH] D37065: Ensure standard pseudo instructions (TargetOpcode::*) are compatible with guessInstructionProperties=0

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: include/llvm/Target/Target.td:818
   let AsmString = "PHINODE";
+  let hasSideEffects = 1;
 }
----------------
qcolombet wrote:
> asb wrote:
> > qcolombet wrote:
> > > I would default hasSideEffects to 0.
> > It seemed odd to me too, but changing it would mean this patch does represent a functional change. I did try setting it to 0 before updating this patch, but found it results in a number of MachineVerifier errors as well as changes in test output.
> Hmm, interesting, I wouldn't have expected this.
> Could you elaborate on what are those?
There are 62 unexpected failures across the default targets. These see either differences in generated code (I haven't examined these), or fail the machine verifier with "Bad machine code: PHI operand is not in the CFG" as well as "Missing PHI operand". For former is triggered by the following logic in MachineVerifier::visitMachineOperand:
```
  case MachineOperand::MO_MachineBasicBlock:
    if (MI->isPHI() && !MO->getMBB()->isSuccessor(MI->getParent()))
      report("PHI operand is not in the CFG", MO, MONum);
```

test/CodeGen/AArch64/swifterror.ll is an example of one of the smaller test cases that fails. It errors with:
```
# After Machine code sinking
# Machine code for function foo_loop: IsSSA, TracksLiveness
Function Live Ins: %X21 in %vreg0, %W0 in %vreg1, %S0 in %vreg2

BB#0: derived from LLVM BB %entry
    Live Ins: %X21 %W0 %S0
	%vreg2<def> = COPY %S0; FPR32:%vreg2
	%vreg1<def> = COPY %W0; GPR32:%vreg1
	%vreg0<def> = COPY %X21; GPR64:%vreg0
	%vreg4<def> = MOVi32imm 16; GPR32:%vreg4
	%vreg5<def> = SUBREG_TO_REG 0, %vreg4, sub_32; GPR64all:%vreg5 GPR32:%vreg4
	%vreg7<def> = MOVi32imm 1; GPR32:%vreg7
	%vreg8<def> = FMOVSi 112; FPR32:%vreg8
    Successors according to CFG: BB#1(?%)

BB#1: derived from LLVM BB %bb_loop
    Predecessors according to CFG: BB#0 BB#3
	CBNZW %vreg1, <BB#2>; GPR32:%vreg1
    Successors according to CFG: BB#2(0x50000000 / 0x80000000 = 62.50%) BB#5(0x30000000 / 0x80000000 = 37.50%)

BB#5: 
    Predecessors according to CFG: BB#1
	%vreg12<def> = PHI %vreg0, <BB#0>, %vreg11, <BB#3>; GPR64all:%vreg12,%vreg11 GPR64:%vreg0
	B <BB#3>
    Successors according to CFG: BB#3(?%)

BB#2: derived from LLVM BB %gen_error
    Predecessors according to CFG: BB#1
	ADJCALLSTACKDOWN 0, 0, %SP<imp-def,dead>, %SP<imp-use>
	%X0<def> = COPY %vreg5; GPR64all:%vreg5
	BL <ga:@malloc>, <regmask %FP %LR %B8 %B9 %B10 %B11 %B12 %B13 %B14 %B15 %D8 %D9 %D10 %D11 %D12 %D13 %D14 %D15 %H8 %H9 %H10 %H11 %H12 %H13 %H14 %H15 %S8 %S9 %S10 %S11 %S12 %S13 %S14 and 57 more...>, %LR<imp-def,dead>, %SP<imp-use>, %X0<imp-use>, %SP<imp-def>, %X0<imp-def>
	ADJCALLSTACKUP 0, 0, %SP<imp-def,dead>, %SP<imp-use>
	%vreg6<def> = COPY %X0; GPR64sp:%vreg6
	%vreg3<def> = COPY %vreg6; GPR64all:%vreg3 GPR64sp:%vreg6
	STRBBui %vreg7, %vreg6, 8; mem:ST1[%tmp] GPR32:%vreg7 GPR64sp:%vreg6
    Successors according to CFG: BB#3(?%)

BB#3: derived from LLVM BB %bb_cont
    Predecessors according to CFG: BB#2 BB#5
	%vreg11<def> = PHI %vreg12, <BB#5>, %vreg3, <BB#2>; GPR64all:%vreg11,%vreg12,%vreg3
	FCMPSrr %vreg2, %vreg8, %NZCV<imp-def>; FPR32:%vreg2,%vreg8
	Bcc 13, <BB#1>, %NZCV<imp-use>
	B <BB#4>
    Successors according to CFG: BB#4(0x04000000 / 0x80000000 = 3.12%) BB#1(0x7c000000 / 0x80000000 = 96.88%)

BB#4: derived from LLVM BB %bb_end
    Predecessors according to CFG: BB#3
	%vreg9<def> = COPY %vreg11; GPR64all:%vreg9,%vreg11
	%vreg10<def> = FMOVS0; FPR32:%vreg10
	%S0<def> = COPY %vreg10; FPR32:%vreg10
	%X21<def> = COPY %vreg9; GPR64all:%vreg9
	RET_ReallyLR %S0<imp-use>, %X21<imp-use>

# End machine code for function foo_loop.

*** Bad machine code: PHI operand is not in the CFG ***
- function:    foo_loop
- basic block: BB#5  (0x2331780)
- instruction: %vreg12<def> = PHI
- operand 2:   <BB#0>

*** Bad machine code: PHI operand is not in the CFG ***
- function:    foo_loop
- basic block: BB#5  (0x2331780)
- instruction: %vreg12<def> = PHI
- operand 4:   <BB#3>

*** Bad machine code: Missing PHI operand ***
- function:    foo_loop
- basic block: BB#5  (0x2331780)
- instruction: %vreg12<def> = PHI
BB#1 is a predecessor according to the CFG.
LLVM ERROR: Found 3 machine code errors.
```

Backends with failing test cases include X86, AArch64, PowerPC, ARM, AMDGPU, Hexagon, and SystemZ. Interestingly, Mips sees no failures.


https://reviews.llvm.org/D37065