Page MenuHomePhabricator

[ARM] isTruncateFree fix
ClosedPublic

Authored by samparker on Sep 25 2017, 5:42 AM.

Details

Summary

Fix the isTruncateFree logic to match my comment, as the previous logic was way too general. Now the only truncates that are free are i64 -> i32. The original patch was in D37516.

Diff Detail

Repository
rL LLVM

Event Timeline

samparker created this revision.Sep 25 2017, 5:42 AM
javed.absar added inline comments.Sep 25 2017, 6:07 AM
lib/Target/ARM/ARMISelLowering.cpp
12336 ↗(On Diff #116540)

maybe factor it out as a separate function to avoid code duplication?

bool ARMTargetLowering::isTruncate64To32(unsigned SrcBits, unsigned DstBits) {

return SrcBits == 64 & DstBits == 32);

}

samparker edited the summary of this revision. (Show Details)Sep 25 2017, 8:09 AM
samparker added inline comments.Sep 25 2017, 8:13 AM
lib/Target/ARM/ARMISelLowering.cpp
12336 ↗(On Diff #116540)

I don't see the value in that here since its a very specific expression. As far as I can see, all the other backends have a similar approach to this as well.

eastig added inline comments.Sep 26 2017, 2:46 AM
lib/Target/ARM/ARMISelLowering.cpp
12336 ↗(On Diff #116540)

I agree with Sam I don't see the big value of ARMTargetLowering::isTruncate64To32.

SjoerdMeijer accepted this revision.Sep 26 2017, 5:17 AM

Looks good to me (sorry I missed this in the other/previous review; I looked at it and thought it was alright...)

This revision is now accepted and ready to land.Sep 26 2017, 5:17 AM
Closed by commit rL314280: [ARM] isTruncateFree fix (authored by sam_parker). · Explain WhySep 27 2017, 1:32 AM
This revision was automatically updated to reflect the committed changes.