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

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1056
+    unsigned CondOpc = CI->getOpcode();
+    if (CI->getValueType(0) == MVT::i1 && CondOpc != ISD::SETCC &&
+        CondOpc != AMDGPUISD::FP_CLASS) {
----------------
arsenm wrote:
> foad wrote:
> > In future it would be nice to improve this to detect a tree of SETCCs connected by ANDs and ORs. This kind of pattern seems to occur quite frequently:
> > ```
> >         v_cmp_ne_u32_e64 s29, s29, 1
> >         v_cmp_ne_u32_e64 s0, s0, 0
> >         s_and_b32 s0, s0, s29
> > +       s_and_b32 s0, exec_lo, s0
> > ```
> > where your patch has added the last AND instruction.
> I thought we had an isVectorBoolean or something like that for identifying these cases recursively (I know I have a patch implementing the equivalent for globalisel lying around at least)
There's this in SIISelLowering.cpp:
```
// Returns true if argument is a boolean value which is not serialized into
// memory or argument and does not require v_cmdmask_b32 to be deserialized.
static bool isBoolSGPR(SDValue V) {
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86878