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

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 17 08:24:07 PDT 2020


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);
----------------
piotr wrote:
> arsenm wrote:
> > 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
> Will do, but what do you mean exactly by "valid boolean sources" here?
I mean like setcc + class intrinsics, other things that will select to VOPC outputs


================
Comment at: llvm/test/CodeGen/AMDGPU/combine-add-zext-xor.ll:16
+
+define void @combine_add_zext_xor() {
+.entry:
----------------
piotr wrote:
> arsenm wrote:
> > Should be able to reduce this more
> This already comes from bugpoint, but I will try to reduce the test a bit more.
Bugpoint isn't that smart and you can usually massage the IR a bit to help it make more progress


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86878



More information about the llvm-commits mailing list