[PATCH] D86878: [AMDGPU] Fix a miscompile with S_ADD/S_SUB

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1056
+    unsigned CondOpc = CI->getOpcode();
+    if (CondOpc == ISD::AND || CondOpc == ISD::OR || CondOpc == ISD::XOR) {
+      auto ST = static_cast<const GCNSubtarget *>(Subtarget);
----------------
I don't think it will end up mattering with the operations legal for i1 (although maybe trunc is also a problem), but I think it would be somewhat safer to list valid boolean sources instead


================
Comment at: llvm/test/CodeGen/AMDGPU/combine-add-zext-xor.ll:16
+
+define void @combine_add_zext_xor() {
+.entry:
----------------
Should be able to reduce this more


================
Comment at: llvm/test/CodeGen/AMDGPU/combine-add-zext-xor.ll:30-31
+1:                                                ; preds = %.exit
+  %2 = or i32 0, %.i112
+  %3 = or i32 0, %2
+  %4 = icmp eq i32 %3, 0
----------------
Should name values (opt -intsnamer)


================
Comment at: llvm/test/CodeGen/AMDGPU/combine-add-zext-xor.ll:59-60
+
+attributes #0 = { nounwind readonly willreturn }
+declare <4 x i32> @llvm.amdgcn.image.load.1d.v4i32.i32(i32 immarg, i32, <8 x i32>, i32 immarg, i32 immarg) #0
----------------
Attributes canonically come after declarations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86878