This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] LegalizationArtifactCombiner: Combine aext([asz]ext x) -> [asz]ext x
ClosedPublic

Authored by volkan on Nov 6 2018, 11:55 AM.

Details

Summary

Replace aext([asz]ext x) with aext/sext/zext x in order to
reduce the number of instructions generated to clean up some
legalization artifacts.

Diff Detail

Repository
rL LLVM

Event Timeline

volkan created this revision.Nov 6 2018, 11:55 AM

What's the reason for not putting this in a post-legalize combiner? It seems to me that we'll have to implement this in the Legalizer and the Combiner

What's the reason for not putting this in a post-legalize combiner? It seems to me that we'll have to implement this in the Legalizer and the Combiner

The main reason is generating a better code by doing this in the legalizer. If we don't handle this in the legalizer, it would be really hard to combine
in a post-legalize combine as LegalizationArtifactCombiner replaces the pattern with shifts and ands.
Here is an example:

%0:_(s32) = COPY $w0
%1:_(s1) = G_TRUNC %0(s32)
%2:_(s8) = G_SEXT %1(s1)
%3:_(s32) = G_ANYEXT %2(s8)
$w0 = COPY %3(s32)

Currently, we generate:

%0:_(s32) = COPY $w0
%13:_(s32) = G_CONSTANT i32 7
%4:_(s8) = G_TRUNC %13(s32)
%10:_(s32) = COPY %0(s32)
%16:_(s32) = G_CONSTANT i32 255
%17:_(s32) = COPY %13(s32)
%11:_(s32) = G_AND %17, %16
%12:_(s32) = G_SHL %10, %11
%18:_(s32) = G_CONSTANT i32 24
%19:_(s32) = COPY %12(s32)
%20:_(s32) = G_SHL %19, %18
%7:_(s32) = G_ASHR %20, %18
%14:_(s32) = G_CONSTANT i32 255
%15:_(s32) = COPY %13(s32)
%8:_(s32) = G_AND %15, %14
%9:_(s32) = G_ASHR %7, %8
%3:_(s32) = COPY %9(s32)
$w0 = COPY %3(s32)

With D54174 applied, we generate:

%0:_(s32) = COPY $w0
%4:_(s32) = G_CONSTANT i32 31
%5:_(s32) = COPY %0(s32)
%6:_(s32) = G_SHL %5, %4
%3:_(s32) = G_ASHR %6, %4
$w0 = COPY %3(s32)

So these extends are created by the legalizer, correct? If so, it seems ok to me as that's what these artifact combines were intended for.

So these extends are created by the legalizer, correct? If so, it seems ok to me as that's what these artifact combines were intended for.

Yes, they are created by the legalizer.

aemerson accepted this revision.Nov 28 2018, 4:21 PM
This revision is now accepted and ready to land.Nov 28 2018, 4:21 PM
This revision was automatically updated to reflect the committed changes.