Page MenuHomePhabricator

[GlobalISel] Combine G_SELECTs of the form (cond ? x : x) into x
ClosedPublic

Authored by paquette on Mar 20 2020, 1:39 PM.

Details

Summary

When we find something like this:

%a:_(s32) = G_SOMETHING ...
...
%select:_(s32) = G_SELECT %cond(s1), %a, %a

We can remove the select and just replace it entirely with %a because it's always going to result in %a.

Same if we have

%select:_(s32) = G_SELECT %cond(s1), %a, %b

where we can deduce that %a == %b.

This implements the following cases:

  • %select:_(s32) = G_SELECT %cond(s1), %a, %a -> %a
  • %select:_(s32) = G_SELECT %cond(s1), %a, %some_copy_from_a -> %a
  • %select:_(s32) = G_SELECT %cond(s1), %a, %b -> %a when %a and %b are defined by identical instructions

This gives a few minor code size improvements on CTMark at -O3 for AArch64.

Diff Detail

Event Timeline

paquette created this revision.Mar 20 2020, 1:39 PM
paquette marked an inline comment as done.Mar 20 2020, 1:41 PM
paquette added inline comments.
llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-select.mir
5–6

also I found out that apparently update_mir_test_checks doesn't like it when this isn't here which is weird. It will skip the function entirely.

arsenm added inline comments.Mar 20 2020, 2:10 PM
llvm/include/llvm/Target/GlobalISel/Combine.td
194

Organize this into an identity_combines?

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-select.mir
5–6

I think there's an off by one. Usually the first line is ---, and ends with ....

paquette updated this revision to Diff 251796.Mar 20 2020, 4:15 PM
paquette marked an inline comment as done.
  • Put the combine into an identity_combines group
  • Fix spot where I wrote MI instead of root
  • Remove extra cruft from test because it just bothers me now
ekatz added a subscriber: ekatz.Mar 20 2020, 8:36 PM

I admit, I don't know much about CG, but shouldn't this optimization be done in the IR level? Or were these SELECTS only introduced in CG?

I admit, I don't know much about CG, but shouldn't this optimization be done in the IR level? Or were these SELECTS only introduced in CG?

Machine combines are mostly for patterns that can appear after legalization. There will always be a lot of overlap with IR optimizations

arsenm added inline comments.Mar 23 2020, 11:59 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
1524–1525

These asserts are both redundant with ones in the accessors

1526

This should specifically be getNumExplicitDefs, this will fail to fail with an implicit def

aemerson accepted this revision.Mar 23 2020, 3:53 PM

LGTM.

This revision is now accepted and ready to land.Mar 23 2020, 3:53 PM

(after Matt's points).

paquette updated this revision to Diff 252175.Mar 23 2020, 4:45 PM

Remove redundant asserts, fix incorrect assert

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2020, 5:27 PM