This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Enhance (zext(setcc))
ClosedPublic

Authored by lkail on Aug 27 2020, 1:19 AM.

Details

Summary

Current v:t = zext(setcc x,y,cc) will be transformed to select x, y, 1:t, 0:t, cc. It misses some opportunities if x's type size is less than t's size. This patch enhances the above transformation.

Diff Detail

Event Timeline

lkail created this revision.Aug 27 2020, 1:19 AM
lkail requested review of this revision.Aug 27 2020, 1:19 AM
lkail edited reviewers, added: steven.zhang; removed: qshanz.
lkail edited the summary of this revision. (Show Details)
xbolva00 added inline comments.
llvm/test/CodeGen/AArch64/hoist-and-by-const-from-lshr-in-eqcmp-zero.ll
312–313

Adjust comment?

spatel added inline comments.Aug 28 2020, 6:56 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
10556

Use 'DAG.getBoolConstant' ?

EVT N0VT = N0.getValueType();
EVT N00VT = N0.getOperand(0).getValueType();
if (SDValue SCC = SimplifySelectCC(
        DL, N0.getOperand(0), N0.getOperand(1),
        DAG.getBoolConstant(true, DL, N0VT, N00VT),
        DAG.getBoolConstant(false, DL, N0VT, N00VT),
        cast<CondCodeSDNode>(N0.getOperand(2))->get(), true))
  return DAG.getNode(ISD::ZERO_EXTEND, DL, VT, SCC);
10561

I think you can delete the existing transform as a special-case - the ZERO_EXTEND node will not be created if VT == N0VT.

lkail updated this revision to Diff 288620.EditedAug 28 2020, 8:43 AM

Per @spatel 's comments.

lkail added inline comments.Aug 28 2020, 8:45 AM
llvm/test/CodeGen/AArch64/hoist-and-by-const-from-lshr-in-eqcmp-zero.ll
312–313

Hmmm, interesting, the function name also indicates it's negative.

lkail edited the summary of this revision. (Show Details)Aug 28 2020, 8:50 AM
spatel added inline comments.Aug 28 2020, 9:00 AM
llvm/test/CodeGen/AArch64/hoist-and-by-const-from-lshr-in-eqcmp-zero.ll
312–313

Yes, please update comment.
https://alive2.llvm.org/ce/z/AEzNaw
Side note: in IR, -instcombine doesn't get this, but -sccp does.

spatel added inline comments.Aug 28 2020, 9:18 AM
llvm/test/CodeGen/AArch64/hoist-and-by-const-from-lshr-in-eqcmp-zero.ll
312–313

"negative" in this context means the test was trying to show that the icmp transform that applies to other tests in this file is not applicable to this test.

lkail updated this revision to Diff 288638.Aug 28 2020, 9:34 AM

Adjust comments, leave function name unchanged.

lkail added inline comments.Aug 28 2020, 9:38 AM
llvm/test/CodeGen/AArch64/hoist-and-by-const-from-lshr-in-eqcmp-zero.ll
312–313

Ok, I see. Looks my latest revision doesn't catch your point. I'll update it.

lkail updated this revision to Diff 288641.Aug 28 2020, 9:46 AM

Updated comments again.

lkail marked 2 inline comments as done.Aug 28 2020, 9:47 AM
spatel accepted this revision.Aug 28 2020, 11:10 AM

LGTM

This revision is now accepted and ready to land.Aug 28 2020, 11:10 AM
This revision was automatically updated to reflect the committed changes.