This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine][WIP] Don't canonicalize select of bools if it costs an instruction
AbandonedPublic

Authored by goldstein.w.n on Aug 31 2023, 6:29 PM.

Details

Reviewers
nikic
RKSimon
Summary

Proofs for new transforms: https://alive2.llvm.org/ce/z/j9o0D3

We current canonicalize true to the TrueVal operand and false to
the FalseVal operand in selects. This makes recognizing LogicalAnd /
LogicalOr patterns easier but often costs an instruction.

Diff Detail

Event Timeline

goldstein.w.n created this revision.Aug 31 2023, 6:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2023, 6:29 PM
goldstein.w.n requested review of this revision.Aug 31 2023, 6:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2023, 6:29 PM
nikic requested changes to this revision.Sep 14 2023, 12:13 AM

I don't think we can do this. Logical and/or is a canonical form that lots of passes rely on, not just InstCombine. We would have to teach all passes that use LogicalAnd/LogicalOr matchers to also support the inverted forms. I don't think that would be worthwhile.

This revision now requires changes to proceed.Sep 14 2023, 12:13 AM

I don't think we can do this. Logical and/or is a canonical form that lots of passes rely on, not just InstCombine. We would have to teach all passes that use LogicalAnd/LogicalOr matchers to also support the inverted forms. I don't think that would be worthwhile.

I don't think we can do this. Logical and/or is a canonical form that lots of passes rely on, not just InstCombine. We would have to teach all passes that use LogicalAnd/LogicalOr matchers to also support the inverted forms. I don't think that would be worthwhile.

I'm happy to drop this. It was mostly motivated by wanting to fix the regression caused by expanding isFreeToInvert.
I'll see if there is another better way to go about it.

goldstein.w.n abandoned this revision.Sep 19 2023, 8:55 AM

Abandoning and resubmitting on GH