SimplifySetCC with immediate integer problem

Code Generation
1

Hello, I found setcc_ge was combined into setcc_gt in my backend by the function:
TargetLowering::SimplifySetCC
The debug log shows below:

Combining: t11: i1 = setcc t2, Constant:i32<-128>, setge:ch
Creating constant: t15: i32 = Constant<-129>
Creating new node: t17: i1 = setcc t2, Constant:i32<-129>, setgt:ch
 ... into: t17: i1 = setcc t2, Constant:i32<-129>, setgt:ch

My problem is I don’t want it combine the setcc_ge into a setcc_gt in this case, because my cmp instructon just support a 8bits signed imm，it just support the imm in the range [-128,127];
I added isLegalICmpImmediate into my TargetLoweringImp, but it dose not stop it, because of the Contant<-128> is not a opaque type: N1C->isOpaque() returned false;

  bool isLegalICmpImmediate(int64_t val) const override {
    return val <= 127 && val >= -128;
  }

// In SimplifySetCC
if ((DCI.isBeforeLegalizeOps() ||
             isCondCodeLegal(NewCC, VT.getSimpleVT())) &&
            (!N1C->isOpaque() || (C.getBitWidth() <= 64 &&
                                  isLegalICmpImmediate(C.getSExtValue())))) {
          return DAG.getSetCC(dl, VT, N0,
                              DAG.getConstant(C, dl, N1.getValueType()),
                              NewCC);
        }

How can I deal with this case to avoid the setcc combine;

1 Like
2

For me it looks like a bug. I think it should have been “if before legalize ops OR (legal cond code AND legal constant) then do the transformation”.

However, this check was added in the very same patch that added the “opaque” flag and ConstantHoisting pass, so it is likely intentional. The history seem to start here.

My best guess is that the check should have been “legal condition AND non-expensive constant AND legal constant” instead of “legal condition AND (non-expensive constant OR legal constant)”, but that caused regressions, so the check was relaxed to assume that legal constants are not expensive.

Either way, the check looks wrong to me - isLegalICmpImmediate should be called on both sides of the “||” (otherwise we are creating an illegal operation, as in your case). Since isLegalICmpImmediate accepts in64_t, this can only be done if getBitWidth() <= 64, which makes “!isOpaque()” call redundant. I.e. in the end it should be “before legalize ops OR (legal cond code AND small constant AND legal constant)”.

I think you can reach wider community by submitting a candidate patch on review.

3

As a workaround, you can write another DAG pattern which matches this specific case, e.g.
def : Pat<(setgt i32:$lhs, -129), (GE $lhs, -128)>;

4

You could use setCondCodeAction to make the CC “custom”, and then deal with it by hand.

5

Ok, It works for me, Thanks;

6

It may more inconvenient than writing DAG patterns for this case, Thank you all the same!

7

I also think this looks like just a bug and you shouldn’t have to workaround with the pattern

8

Yes, I have submited a issue: SimplifySetCC with immediate integer problem · Issue #56666 · llvm/llvm-project · GitHub