Page MenuHomePhabricator

[InstCombine] Allow fptrunc (fpext X)) to be reduced to a single fpext/ftrunc
ClosedPublic

Authored by craig.topper on Mar 1 2018, 1:55 PM.

Details

Summary

If we are only removing bits that were added by the extend we should be able to use a smaller extend. I think.

We already reduce cases like this when there is binop in between. For example:

define float @bar(half %a, half %x) {
    %b = fpext half %a to double
    %z = fpext half %x to double
    %y = fmul double %b, %z
    %c = fptrunc double %y to float
    ret float %c
}

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Mar 1 2018, 1:55 PM
scanon added a comment.Mar 1 2018, 2:00 PM

We should be able to go farther and just do a fptrunc if SrcSize > DstSize.

craig.topper retitled this revision from [InstCombine] Allow fptrunc (fpext X)) to be reduced to a single fpext if the trunc result type is larger than the original size to [InstCombine] Allow fptrunc (fpext X)) to be reduced to a single fpext/ftrunc.

Allow it to be turned into fptrunc as well. Which I believe makes it equivalent to the handler for integers.

Add directed tests

Does this do the right thing for a trunc/ext sequence converting from ppc_fp128 to fp128?

Probably not, but I don't know how to create that case without an intermediate type larger than 128-bits. Unless I'm missing something.

Oh, yes, you're right; not sure what I was thinking.

scanon added inline comments.Mar 2 2018, 8:45 AM
lib/IR/Instructions.cpp
2270 ↗(On Diff #136621)

I don't think that this comment clarifies anything, since case 10 no longer exists. It would probably be clearer to just remove it; the repository history documents what it "used to be".

Remove comment and rebase

scanon accepted this revision.Mar 2 2018, 10:02 AM

LGTM.

This revision is now accepted and ready to land.Mar 2 2018, 10:02 AM
This revision was automatically updated to reflect the committed changes.