This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Simplify setcc (sext from i1 b), -1|0, cc
ClosedPublic

Authored by rampitec on Jun 22 2017, 5:29 PM.

Details

Summary

Depending on the compare code that can be either an argument of
sext or negate of it. This helps to avoid v_cndmask_b64 instruction
for sext. A reversed value can be further simplified and folded into
its parent comparison if possible.

Diff Detail

Repository
rL LLVM

Event Timeline

rampitec created this revision.Jun 22 2017, 5:29 PM
arsenm edited edge metadata.Jun 22 2017, 5:33 PM

There are already an extensive number of combines that do this sort of thing in the generic DAGcombiner. Have you looked into why those aren't working? I know some are broken because they don't properly check the TargetBooalenContents

There are already an extensive number of combines that do this sort of thing in the generic DAGcombiner. Have you looked into why those aren't working? I know some are broken because they don't properly check the TargetBooalenContents

These patterns are quite specific to our way of bool deserialization. Moreover, just like previous it is not always beneficial to do, it all depends where do we expect to have initial bool value.

arsenm added inline comments.Jun 22 2017, 5:57 PM
lib/Target/AMDGPU/SIISelLowering.cpp
4315 ↗(On Diff #103672)

Deserialized isn't the right word. I think you mean an i1 which must be an in SGPR/condition register. Maybe isBoolSGPR or something?

5127–5135 ↗(On Diff #103672)

Constants are canonicalized to the RHS as is, you shouldn't' need to handle this here

5137 ↗(On Diff #103672)

CRHS condition first

arsenm added inline comments.Jun 22 2017, 6:04 PM
lib/Target/AMDGPU/SIISelLowering.cpp
5143 ↗(On Diff #103672)

I don't think it is useful check the GT/LT cases. I would expect those to fold to eq/ne the bool value already

rampitec added inline comments.Jun 22 2017, 6:05 PM
lib/Target/AMDGPU/SIISelLowering.cpp
5127–5135 ↗(On Diff #103672)

I agree, but I have seen a real case where it was swapped.

rampitec added inline comments.Jun 22 2017, 6:09 PM
lib/Target/AMDGPU/SIISelLowering.cpp
5143 ↗(On Diff #103672)

It is not folded. At least not always.

rampitec updated this revision to Diff 103680.Jun 22 2017, 6:12 PM
rampitec marked 2 inline comments as done.

Renamed function and changed check order.

vpykhtin accepted this revision.Jun 27 2017, 3:05 AM

LGTM.

This revision is now accepted and ready to land.Jun 27 2017, 3:05 AM
This revision was automatically updated to reflect the committed changes.