Page MenuHomePhabricator

[InstCombine] Canonicalize widenable.conditions for ease of pattern matching (and cases)
Needs ReviewPublic

Authored by reames on Nov 20 2019, 6:16 PM.



Canonicalize and expressions involving widenable.conditions in two ways:

  1. Treat widenable.conditions are more complex (and thus placed on the LHS) as compared to other instructions
  2. Sink widenable.conditions to the outermost and in any and-tree.
  3. Remove redundant widenable.conditions within a single and-tree.

The net effect is that we merge two widenable branches (say via SimplifyCFG realizing they share a slowpath branch), we can canonicalize back to a simple widenable branch with some set of conditions and a single WC call.

Note that this needs to land after D70502 as it swaps the current right operand preference for the WC. The current choice is problematic as instcombine will happily swap operands such that e.g. an argument is on the right instead.

Diff Detail

Event Timeline

reames created this revision.Nov 20 2019, 6:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 20 2019, 6:17 PM
reames updated this revision to Diff 230363.Nov 20 2019, 6:35 PM

(add context to diff)

ebrevnov added inline comments.Nov 21 2019, 9:44 PM

It's hard for me to match this comment to real code. Maybe just say "Swap OP1 & Op2 if Op2 is WC and Op1 is not" or "Canonicalize WC to LHS if RHS is not WC"?


That will change evaluation order between A and B.


This comment confused me a bit. My initial understanding was that next 'if' should handle this case . It took me a while to understand that it is actually handled by one of the previous 'if'. May be reformulate in terms:
A can't be WC because...
B can't be WC because...


??? C can't be WC because...???