This is an archive of the discontinued LLVM Phabricator instance.

[DAG] computeKnownBits - add ISD::MULHS/MULHU/SMUL_LOHI/UMUL_LOHI handling
ClosedPublic

Authored by RKSimon on Mar 18 2021, 5:15 AM.

Details

Summary

Reuse the existing KnownBits multiplication code to handle the 'extend + multiply + extract high bits' pattern for multiply-high ops.

Noticed while looking at the codegen for D88785 / D98587 - the patch helps division-by-constant expansion code in particular, which suggests that we might have some further KnownBits div/rem cases we could handle - but this was far easier to implement.

Diff Detail

Event Timeline

RKSimon created this revision.Mar 18 2021, 5:15 AM
RKSimon requested review of this revision.Mar 18 2021, 5:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 18 2021, 5:15 AM
Herald added a subscriber: wdng. · View Herald Transcript

Can you please split this into KnownBits patch and SelectionDAG patch?

Can you please split this into KnownBits patch and SelectionDAG patch?

Sure

foad added inline comments.Mar 18 2021, 9:40 AM
llvm/include/llvm/Support/KnownBits.h
299 ↗(On Diff #331519)

Typo "extened"

302 ↗(On Diff #331519)

Zero-extended?

RKSimon updated this revision to Diff 331714.Mar 18 2021, 3:55 PM

rebase on top of D98866

I think you may be able to quite easily add a case for S/UMUL_LOHI here as well (but unsure if its necessary, just noticing that its not explicitly handled here)

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
2983

What does this (and a similar one below) comment signify?

I think you may be able to quite easily add a case for S/UMUL_LOHI here as well (but unsure if its necessary, just noticing that its not explicitly handled here)

I'm happy to do that (and MULHS NumSignBits) as followup patches.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
2983

It was of more use when I did the expansion here instead of inside KnownBits - I'll drop it

RKSimon updated this revision to Diff 331811.Mar 19 2021, 3:10 AM

remove old comment

RKSimon updated this revision to Diff 331878.Mar 19 2021, 7:56 AM
RKSimon retitled this revision from [DAG] computeKnownBits - add ISD::MULHS/ISD::MULHU handling to [DAG] computeKnownBits - add ISD::MULHS/MULHU/SMUL_LOHI/UMUL_LOHI handling.

Added *MUL_LOHI handling as the diff was relatively benign

foad accepted this revision.Mar 19 2021, 8:02 AM

The code changes look good to me, and the effect on the the AMDGPU tests is nice.

This revision is now accepted and ready to land.Mar 19 2021, 8:02 AM
This revision was landed with ongoing or failed builds.Mar 19 2021, 9:03 AM
This revision was automatically updated to reflect the committed changes.