anoopkg6 wrote:
>Why restrict this to SELECT_CCMASK? If the transformation is correct for
>SELECT_CCMASK, it must also be correct for >BR_CCMASK.
We combine select_ccmask in combineCCMask from the call combineSELECT_CCMASK
and combineBR_CCMASK.
But not the case combining select_ccmask with br_ccmask (select_ccmask
(br_ccmask)) or br_ccmask with br_ccmask (br_ccmask (br_ccmask)).
>CCNode cannot be SRL - SRL returns a GPR, and CCNode returns a condition code.
>Therefore this check can never hit.
I think CCNode name should be changed. It is not returning anything, just
checking (srl (ipm) pattern. It is being used as follows.
>I don't know what this branch is supposed to optimize, but I notice that
>within that whole if block, you not once even refer to >AndOp1 - so every (and
>(select_ccmask) XXX) is optimized to the same thing, no matter what XXX is.
>That obviously cannot be >correct.
There are several cases where CC is directly being used. One interesting
example.
CC == 0 || CC == 2 || CC == 3.
SelectionDAG has 29 nodes:
t0: ch,glue = EntryToken
t8: ch,glue = inlineasm t0, TargetExternalSymbol:i64' alsi $1,-1
', MDNode:ch<null>, TargetConstant:i64<25>, TargetConstant:i32<458762>,
Register:i32 %0, TargetConstant:i32<524302>, t42, TargetConstant:i32<524302>,
t42
t10: i32,ch,glue = CopyFromReg t8, Register:i32 $cc, t8:1
t11: i32 = SystemZISD::IPM t10
t13: i32 = srl t11, Constant:i32<28>
t42: i64 = SystemZISD::PCREL_WRAPPER TargetGlobalAddress:i64<ptr @a> 0
t37: i32 = SystemZISD::ICMP t13, Constant:i32<3>,
TargetConstant:i32<0>
t40: i32 = SystemZISD::SELECT_CCMASK Constant:i32<1>,
Constant:i32<0>, TargetConstant:i32<14>, TargetConstant:i32<6>, t37
t28: i32 = and t40, t13
t43: i32 = SystemZISD::ICMP t28, Constant:i32<0>, TargetConstant:i32<0>
t44: ch = SystemZISD::BR_CCMASK t10:1, TargetConstant:i32<14>,
TargetConstant:i32<6>, BasicBlock:ch<if.end 0x9f61ff0>, t43
t25: ch = br t44, BasicBlock:ch<if.then 0x9f61ed0>
________________________________
From: Ulrich Weigand ***@***.***>
Sent: Monday, June 30, 2025 6:32 AM
To: llvm/llvm-project ***@***.***>
Cc: Anoop Kumar ***@***.***>; Author ***@***.***>
Subject: [EXTERNAL] Re: [llvm/llvm-project] Add support for flag output operand
***@***.***" for SystemZ. (PR #125970)
@ uweigand commented on this pull request. In
llvm/lib/Target/SystemZ/SystemZISelLowering. cpp: > + // Optimizing only the
case where Op0TrueVal and Op1TrueVal are equal + // and at the same time
Op0FalseVal and Op1FalseVal are also equal.
@uweigand commented on this pull request.
________________________________
In
llvm/lib/Target/SystemZ/SystemZISelLowering.cpp<https://github.com/llvm/llvm-project/pull/125970#discussion_r2174845321
>:
> + // Optimizing only the case where Op0TrueVal and Op1TrueVal are equal
+ // and at the same time Op0FalseVal and Op1FalseVal are also equal.
+ auto *Op0TrueVal = dyn_cast<ConstantSDNode>(AndOp0->getOperand(0));
+ auto *Op0FalseVal = dyn_cast<ConstantSDNode>(AndOp0->getOperand(1));
+ auto *Op1TrueVal = dyn_cast<ConstantSDNode>(AndOp1->getOperand(0));
+ auto *Op1FalseVal = dyn_cast<ConstantSDNode>(AndOp1->getOperand(1));
+ if (!Op0TrueVal || !Op0FalseVal || !Op1TrueVal || !Op1FalseVal)
+ return SDValue();
+ if (Op0TrueVal->getZExtValue() != Op1TrueVal->getZExtValue() ||
+ Op0FalseVal->getZExtValue() != Op1FalseVal->getZExtValue())
+ return SDValue();
+
+ // Compute the effective CC mask for select.
+ int Op0CCMaskVal = Op0CCMask->getZExtValue();
+ int Op1CCMaskVal = Op1CCMask->getZExtValue();
+ int CCMask = Op0CCMaskVal & Op1CCMaskVal;
This still isn't correct. Looking at the options for the AND case, we have in
the general case:
0) CC in CCMASK1 and CCMASK2 --> (and TRUEVAL1 TRUEVAL2)
1) CC in CCMASK1 and not in CCMASK2 --> (and TRUEVAL1 FALSEVAL2)
2) CC not in CCMASK1 but in CCMASK2 --> (and FALSEVAL1 TRUEVAL2)
3) CC neither in CCMASK1 nor in CCMASK2 -> (and FALSEVAL1 FALSEVAL2)
Now, even if you assume (and check for) TRUEVAL1 == TRUEVAL2 and FALSEVAL1 ==
FALSEVAL2, we still have three options:
0) CC in CCMASK1 and CCMASK2 --> TRUEVAL
1/2) CC in one but not both of CCMASK1 and CCMASK2 --> (and TRUEVAL FALSEVAL)
3) CC neither in CCMASK1 nor in CCMASK2 -> FALSEVAL
In order to get down to two options that can be represented by a single
SELECT_CCMASK, you have to impose even more constraints.
________________________________
In
llvm/lib/Target/SystemZ/SystemZISelLowering.cpp<https://github.com/llvm/llvm-project/pull/125970#discussion_r2174852626
>:
> + for (SDUse &SelectBrUse : Icmp->uses()) {
+ auto *SelectBr = SelectBrUse.getUser();
+ if (SelectBr && (SelectBr->getOpcode() == SystemZISD::SELECT_CCMASK
||
+ SelectBr->getOpcode() == SystemZISD::BR_CCMASK))
+ DCI.AddToWorklist(SelectBr);
+ }
+ }
+ }
+ return DAG.getNode(SystemZISD::SELECT_CCMASK, SDLoc(N), N->getValueType(0),
+ AndOp0->getOperand(0), AndOp0->getOperand(1),
+ DAG.getTargetConstant(Op0CCValidVal, SDLoc(N),
MVT::i32),
+ DAG.getTargetConstant(CCMask, SDLoc(N), MVT::i32),
+ Op0CCReg);
+ } else if (AndOp0->getOpcode() == SystemZISD::SELECT_CCMASK) {
+ // AndOp1: (SRL (IPM (CC))).
+ // AndOp2: CC.
I don't know what this branch is supposed to optimize, but I notice that within
that whole if block, you not once even refer to AndOp1 - so every (and
(select_ccmask) XXX) is optimized to the same thing, no matter what XXX is.
That obviously cannot be correct.
________________________________
In
llvm/lib/Target/SystemZ/SystemZISelLowering.cpp<https://github.com/llvm/llvm-project/pull/125970#discussion_r2174853599
>:
> + // Optimizing only the case where Op0TrueVal and Op1TrueVal are equal
+ // and at the same time Op0FalseVal and Op1FalseVal are also equal.
+ auto *Op0TrueVal = dyn_cast<ConstantSDNode>(OrOp0->getOperand(0));
+ auto *Op0FalseVal = dyn_cast<ConstantSDNode>(OrOp0->getOperand(1));
+ auto *Op1TrueVal = dyn_cast<ConstantSDNode>(OrOp1->getOperand(0));
+ auto *Op1FalseVal = dyn_cast<ConstantSDNode>(OrOp1->getOperand(1));
+ if (!Op0TrueVal || !Op0FalseVal || !Op1TrueVal || !Op1FalseVal)
+ return SDValue();
+ if (Op0TrueVal->getZExtValue() != Op1TrueVal->getZExtValue() ||
+ Op0FalseVal->getZExtValue() != Op1FalseVal->getZExtValue())
+ return SDValue();
+
+ // Compute the effective CC mask for select.
+ int Op0CCMaskVal = Op0CCMask->getZExtValue();
+ int Op1CCMaskVal = Op1CCMask->getZExtValue();
+ int CCMask = Op0CCMaskVal | Op1CCMaskVal;
Same as above, this is not correct.
________________________________
In
llvm/lib/Target/SystemZ/SystemZISelLowering.cpp<https://github.com/llvm/llvm-project/pull/125970#discussion_r2174854033
>:
> + // Optimizing only the case where Op0TrueVal and Op1TrueVal are equal
+ // and at the same time Op0FalseVal and Op1FalseVal are also equal.
+ auto *Op0TrueVal = dyn_cast<ConstantSDNode>(XorOp0->getOperand(0));
+ auto *Op0FalseVal = dyn_cast<ConstantSDNode>(XorOp0->getOperand(1));
+ auto *Op1TrueVal = dyn_cast<ConstantSDNode>(XorOp1->getOperand(0));
+ auto *Op1FalseVal = dyn_cast<ConstantSDNode>(XorOp1->getOperand(1));
+ if (!Op0TrueVal || !Op0FalseVal || !Op1TrueVal || !Op1FalseVal)
+ return SDValue();
+ if (Op0TrueVal->getZExtValue() != Op1TrueVal->getZExtValue() ||
+ Op0FalseVal->getZExtValue() != Op1FalseVal->getZExtValue())
+ return SDValue();
+
+ // Compute the effective CC mask for select.
+ int Op0CCMaskVal = Op0CCMask->getZExtValue();
+ int Op1CCMaskVal = Op1CCMask->getZExtValue();
+ int CCMask = Op0CCMaskVal ^ Op1CCMaskVal;
Same here.
________________________________
In
llvm/lib/Target/SystemZ/SystemZISelLowering.cpp<https://github.com/llvm/llvm-project/pull/125970#discussion_r2174857498
>:
> + auto *SRLCount = dyn_cast<ConstantSDNode>(N->getOperand(1));
+ if (!SRLCount || SRLCount->getZExtValue() != SystemZ::IPM_CC)
+ return false;
+ auto *IPM = N->getOperand(0).getNode();
+ if (!IPM || IPM->getOpcode() != SystemZISD::IPM)
+ return false;
+ CCReg = IPM->getOperand(0);
+ return true;
+ };
+
+ auto *CCNode = CCReg.getNode();
+ if (!CCNode)
+ return false;
+
+ // Check (SRL (IPM)) pattern and update CCReg if true.
+ if (isSRL_IPM_CCSequence(CCNode))
CCNode cannot be SRL - SRL returns a GPR, and CCNode returns a condition code.
Therefore this check can never hit.
________________________________
In
llvm/lib/Target/SystemZ/SystemZISelLowering.cpp<https://github.com/llvm/llvm-project/pull/125970#discussion_r2174858284
>:
> + if (!IPM || IPM->getOpcode() != SystemZISD::IPM)
+ return false;
+ CCReg = IPM->getOperand(0);
+ return true;
+ };
+
+ auto *CCNode = CCReg.getNode();
+ if (!CCNode)
+ return false;
+
+ // Check (SRL (IPM)) pattern and update CCReg if true.
+ if (isSRL_IPM_CCSequence(CCNode))
+ return true;
+
+ // Combine CCMASK_TM with select_ccmask.
+ if (CCNode->getOpcode() == SystemZISD::SELECT_CCMASK) {
Why restrict this to SELECT_CCMASK? If the transformation is correct for
SELECT_CCMASK, it must also be correct for BR_CCMASK.
—
Reply to this email directly, view it on
GitHub<https://github.com/llvm/llvm-project/pull/125970#pullrequestreview-2970867663
>, or
unsubscribe<https://github.com/notifications/unsubscribe-auth/BM5K4GSUCCTFRKHBDXETY5D3GEN5DAVCNFSM6AAAAABWSJVEQKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDSNZQHA3DONRWGM
>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
https://github.com/llvm/llvm-project/pull/125970
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
