Page MenuHomePhabricator

[GlobalISel][AArch64] Use getConstantVRegValWithLookThrough for selectArithImmed
ClosedPublic

Authored by paquette on Jul 2 2019, 3:51 PM.

Details

Summary

Look through G_TRUNCs, G_SEXTs, and G_ZEXTs when looking for a G_CONSTANT instead of giving up when the first thing we see isn't a G_CONSTANT.

This gives an average ~1.3% code size improvement on CINT2000 at -O3.

Diff Detail

Repository
rL LLVM

Event Timeline

paquette created this revision.Jul 2 2019, 3:51 PM
qcolombet requested changes to this revision.Jul 2 2019, 4:08 PM
qcolombet added a subscriber: qcolombet.

General direction LGTM but there's a bug in the current patch unless I am mistaken.

As a general note, if we need to use this specific helper in more places, I think we should fix the way we handle constant like I already mentioned in https://reviews.llvm.org/D59227.

llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
3672 ↗(On Diff #207646)

You can get the value directly with ValAndVReg->Value, instead of checking for the G_CONSTANT and such.
This is actually better that ZExt the value yourself because the constant that you're reading could have been sext or truncated, etc. and you're code misses that.

llvm/test/CodeGen/AArch64/GlobalISel/select-cmp.mir
90 ↗(On Diff #207646)

Could you write a test that would make the actual 64-bit constant to drop some bits when truncated to 32-bit?

That way that would catch the bug I was mentioning in this patch.

This revision now requires changes to proceed.Jul 2 2019, 4:08 PM
paquette updated this revision to Diff 207655.Jul 2 2019, 4:39 PM

Use getConstantVRegValWithLookThrough properly and update test to include a weird number which should lose some bits in the G_TRUNC.

paquette updated this revision to Diff 207817.Jul 3 2019, 9:31 AM

Update test with a value that selects to the immediate form.

qcolombet accepted this revision.Jul 3 2019, 9:42 AM
This revision is now accepted and ready to land.Jul 3 2019, 9:42 AM
This revision was automatically updated to reflect the committed changes.