[AMDGPU] Fixed handling of imemdiate i1 literals

[rampitec]

Summary

This bug was exposed by the rL360395.

[arsenm]

lib/Target/AMDGPU/SIInstrInfo.cpp

2526 ↗

(On Diff #199107)

I don’t understand where this is coming from. There should be no 1-bit immediates anywhere?

[rampitec]

lib/Target/AMDGPU/SIInstrInfo.cpp

2526 ↗

(On Diff #199107)

Combiner sometimes produces "xor x, true", and even "add x, true". This is not the first time we hit it, we have even implemented lowering.

[nhaehnle]

Why does this return false? A 1-bit immediate is either 0 or -1, both of which can be represented as inline constants everywhere.

[arsenm]

lib/Target/AMDGPU/SIInstrInfo.cpp

2526 ↗

(On Diff #199107)

I would expect this to be extended to the wavesize during selection

2526 ↗

(On Diff #199107)

I guess that wouldn't provide much benefit, but this should always be true?

[michel.daenzer]

FWIW, this fixes the regression I reported, without regressing any other tests I run.

[rampitec]

In D61812#1499954, @nhaehnle wrote:

Why does this return false? A 1-bit immediate is either 0 or -1, both of which can be represented as inline constants everywhere.

Technically yes, but the query is about VOP literal, while i1 ends up in a SOP normally.

lib/Target/AMDGPU/SIInstrInfo.cpp

2526 ↗

(On Diff #199107)

Yes, these are extended, but we have the situation where predicate is checked before the extension.

While it technically can fit VOP literal I do not believe that would be a good idea. The bool operand shall go SOP instruction, so I preffered to return false. At the very least we will cancel unsuitable pattern matching earlier this way.

[arsenm]

lib/Target/AMDGPU/SIInstrInfo.cpp

2526 ↗

(On Diff #199107)

But SOP instructions have the same inline immediate values. This isn't a property of the immediate or instruction

[arsenm]

lib/Target/AMDGPU/SIInstrInfo.cpp

2526 ↗

(On Diff #199107)

OK, I see this is breaking not at the point I thought. In the bool case these still end up materialized with regular int64_t operands. This is only happening in the DAG patterns. Saying this is true should be fine, since it still works with the normal immediate folding heuristics.

[rampitec]

Changed return to true.

[arsenm]

LGTM

test/CodeGen/AMDGPU/xor3-i1-const.ll

3 ↗

(On Diff #199463)

Grammar

9–19 ↗

(On Diff #199463)

This could probably be reduced a bit

[rampitec]

test/CodeGen/AMDGPU/xor3-i1-const.ll

9–19 ↗

(On Diff #199463)

I was unable to reduce it further. Combine conditions are peculiar.

llvm/trunk/lib/Target/AMDGPU/SIInstrInfo.cpp

bool SIInstrInfo::mayReadEXEC(const MachineRegisterInfo &MRI,
   if (!isTargetSpecificOpcode(MI.getOpcode()))
     return true;
 
   return !isSALU(MI) || MI.readsRegister(AMDGPU::EXEC, &RI);
 }
 
 bool SIInstrInfo::isInlineConstant(const APInt &Imm) const {
   switch (Imm.getBitWidth()) {
+  case 1: // This likely will be a condition code mask.
+    return true;
+
   case 32:
     return AMDGPU::isInlinableLiteral32(Imm.getSExtValue(),
                                         ST.hasInv2PiInlineImm());
   case 64:
     return AMDGPU::isInlinableLiteral64(Imm.getSExtValue(),
                                         ST.hasInv2PiInlineImm());
   case 16:
     return ST.has16BitInsts() &&

llvm/trunk/test/CodeGen/AMDGPU/xor3-i1-const.ll

+; RUN: llc -march=amdgcn -mcpu=bonaire -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
+
+; This test used to crash
+; GCN-LABEL: {{^}}xor3_i1_const:
+; GCN: s_xor_b64 s[{{[0-9:]+}}], s[{{[0-9:]+}}], -1
+; GCN: s_xor_b64 s[{{[0-9:]+}}], s[{{[0-9:]+}}], -1
+define amdgpu_ps float @xor3_i1_const(float inreg %arg1, i32 inreg %arg2) {
+main_body:
+  %tmp26 = fcmp nsz olt float %arg1, 0.000000e+00
+  %tmp28 = call nsz float @llvm.amdgcn.interp.p2(float undef, float undef, i32 0, i32 0, i32 %arg2)
+  %tmp29 = fcmp nsz olt float %arg1, 5.700000e+01
+  %tmp31 = fcmp nsz olt float %tmp28, 0.000000e+00
+  %.demorgan = and i1 %tmp26, %tmp29
+  %tmp34 = xor i1 %.demorgan, true
+  %tmp35 = and i1 %tmp31, %tmp34
+  %tmp36 = xor i1 %tmp35, true
+  %tmp37 = xor i1 %.demorgan, %tmp36
+  %tmp42 = or i1 %tmp37, %tmp35
+  %tmp43 = select i1 %tmp42, float 1.000000e+00, float 0.000000e+00
+  ret float %tmp43
+}
+
+declare float @llvm.amdgcn.interp.p2(float, float, i32, i32, i32)