-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[RISCV] Fix vmerge.vvm/vmv.v.v getting folded into ops with mismatching EEW #101152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3855,11 +3855,19 @@ bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) { | |
// If we end up changing the VL or mask of True, then we need to make sure it | ||
// doesn't raise any observable fp exceptions, since changing the active | ||
// elements will affect how fflags is set. | ||
if (TrueVL != VL || !IsMasked) | ||
if (TrueVL != VL || !IsMasked) { | ||
if (mayRaiseFPException(True.getNode()) && | ||
!True->getFlags().hasNoFPExcept()) | ||
return false; | ||
|
||
// If the EEW of True is different from vmerge's SEW, then we cannot change | ||
// the VL or mask. | ||
if (Log2_64(True.getScalarValueSizeInBits()) != | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe don't use log here, we do a shift left to |
||
N->getConstantOperandVal( | ||
RISCVII::getSEWOpNum(TII->get(N->getMachineOpcode())) - 1)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The opcode is known? So I think we don't need to get SEW operand index from TSFlag, we can hardcode it here. |
||
return false; | ||
} | ||
|
||
SDLoc DL(N); | ||
|
||
// From the preconditions we checked above, we know the mask and thus glue | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is specific to the change case. Even if we have exactly equal VL values, those values describe a different number of bits. So folding away the vmerge.vv and vmv.v.v is still illegal.
I think you can also use a much easier check here - the VT of the TrueOp should equal the VT of the vmerge or vmv.v.v. (Really the respective operand, but we don't have widening or narrowing versions of either so that's equivalent.)