This is an archive of the discontinued LLVM Phabricator instance.

[Instsimplfy] X == Y ? 0 : X - Y --> X - Y
ClosedPublic

Authored by junaire on May 11 2023, 8:47 AM.

Diff Detail

Event Timeline

junaire created this revision.May 11 2023, 8:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2023, 8:47 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
junaire requested review of this revision.May 11 2023, 8:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2023, 8:47 AM
goldstein.w.n added inline comments.May 11 2023, 9:55 AM
llvm/lib/Analysis/InstructionSimplify.cpp
4585

While you're at it, can you also do select (X != Y ? X - Y : 0)? Something like:

if(CMPEQ || CMPNE) {
Value * MatchZero, *MatchSub;
if(CMPEQ) {
  MatchZero = TrueVal;
  MatchSub = FalseVal;
}
else {
  MatchZero = FalseVal;
  MatchSub = TrueVal;
}

// Your current code using MatchZero/MatchSub.
}

You will have to move out of the current branch.

4591

can you put this all in nested if (match(TrueVal, m_Zero)) { // check RHS-LHS and LHS-RHS }

nikic added a comment.May 11 2023, 2:00 PM

The handling for this should be inside simplifyWithOpReplaced().

junaire updated this revision to Diff 521556.May 11 2023, 10:08 PM

Move the fold to simplifyWithOpReplaced

junaire marked 2 inline comments as done.May 11 2023, 10:12 PM

The handling for this should be inside simplifyWithOpReplaced().

I updated the patch. The transform I wrote is based on my understanding of the code and I'm not super confident that's what you meant. Educate me please if it's wrong, thanks!

llvm/lib/Analysis/InstructionSimplify.cpp
4585

NE will always be canonicalized to EQ in Line 4476, so we don't have to do that.

goldstein.w.n added inline comments.May 11 2023, 10:16 PM
llvm/lib/Analysis/InstructionSimplify.cpp
4252

I think this can just return zero.

junaire updated this revision to Diff 521558.May 11 2023, 10:21 PM
junaire marked an inline comment as done.

Replace simplifyInstructionWithOperands => Constant::getNullValue(Op0->getType())

junaire marked an inline comment as done.May 11 2023, 10:21 PM
junaire added inline comments.
llvm/lib/Analysis/InstructionSimplify.cpp
4252

Done, thx.

junaire marked an inline comment as done.May 11 2023, 10:22 PM
nikic accepted this revision.May 16 2023, 2:55 AM

LGTM

I've clarified the preconditions for this function in https://github.com/llvm/llvm-project/commit/8d2bae8c227debdcd0632ce364c58883bd12ad84, so we can actually justify the transform.

llvm/lib/Analysis/InstructionSimplify.cpp
4250–4252
This revision is now accepted and ready to land.May 16 2023, 2:55 AM
junaire updated this revision to Diff 522533.May 16 2023, 4:10 AM

Address comments, thanks!

This revision was landed with ongoing or failed builds.May 16 2023, 4:16 AM
This revision was automatically updated to reflect the committed changes.