This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add custom type legalization to form MULHSU when possible.
ClosedPublic

Authored by craig.topper on Mar 28 2021, 5:20 PM.

Details

Summary

There's no target independent ISD opcode for MULHSU, so custom
legalize 2*XLen multiplies ourselves. We have to be a little
careful to prefer MULHU or MULHSU.

I thought about doing this in isel by pattern matching the
(add (mul X, (srai Y, XLen-1)), (mulhu X, Y)) pattern. I decided
against this because the add might become part of a chain of adds.
I don't trust DAG combine not to reassociate with other adds making
it difficult to find both pieces again.

Diff Detail

Unit TestsFailed

Event Timeline

craig.topper created this revision.Mar 28 2021, 5:20 PM
craig.topper requested review of this revision.Mar 28 2021, 5:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 28 2021, 5:20 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
asb accepted this revision.Apr 1 2021, 4:42 AM

LGTM.

I think it's OK to land without this, but: Running this patch quickly across the GCC torture suite, the only codegen changes are to 20120817-1.c and pr51581-{1,2}.c on RV32 where a mulhsu is now selected rather than a solitary mulh. I believe this is still correct, but it made me wonder if we are missing some test coverage and should have a codegen test that would demonstrate that change.

This revision is now accepted and ready to land.Apr 1 2021, 4:42 AM
In D99479#2663549, @asb wrote:

LGTM.

I think it's OK to land without this, but: Running this patch quickly across the GCC torture suite, the only codegen changes are to 20120817-1.c and pr51581-{1,2}.c on RV32 where a mulhsu is now selected rather than a solitary mulh. I believe this is still correct, but it made me wonder if we are missing some test coverage and should have a codegen test that would demonstrate that change.

I didn't intend that to happen. I was hoping to only affect cases that couldn't be handled by MULH and MULHU. I'll see about making my checks stricter.

This revision was landed with ongoing or failed builds.Apr 1 2021, 10:16 AM
This revision was automatically updated to reflect the committed changes.