This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Don't advertise i32->i64 zextload as free for RV64.
ClosedPublic

Authored by craig.topper on Jan 3 2022, 4:39 PM.

Details

Summary

The zextload hook is only used to determine whether to insert a
zero_extend or any_extend for narrow types leaving a basic block.
Returning true from this hook tends to cause any load whose output
leaves the basic block to become an LWU instead of an LW.

Since we tend to prefer sexts for i32 compares on RV64, this can
cause extra sext.w instructions to be created in other basic blocks.

If we use LW instead of LWU this gives the MIR pass from D116397
a better chance of removing them.

Another option might be to teach getPreferredExtendForValue in
FunctionLoweringInfo.cpp about our preference for sign_extend of
i32 compares. That would cause SIGN_EXTEND to be chosen for any
value used by a compare instead of using the isZExtFree heuristic.
That will require code to convert from the llvm::Type* to EVT/MVT
as well as querying the type legalization actions to get the
promoted type in order to call TargetLowering::isSExtCheaperThanZExt.
That seemed like many extra steps when no other target wants it.
Though it would avoid us needing to lean on the MIR pass in some cases.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 3 2022, 4:39 PM
craig.topper requested review of this revision.Jan 3 2022, 4:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2022, 4:39 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
asb accepted this revision.Jan 6 2022, 7:17 AM

I think in principle, leaving isZExtFree as-is and making other changes to resolve the issue around compares is better. But that's quite a lot more work, and this patch solves a real problem, so I'm happy to go with the pragmatic option, landing this patch as-is.

This revision is now accepted and ready to land.Jan 6 2022, 7:17 AM
This revision was landed with ongoing or failed builds.Jan 6 2022, 8:16 AM
This revision was automatically updated to reflect the committed changes.