This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] reverse bitcast + bitwise-logic canonicalization (PR33138)
ClosedPublic

Authored by spatel on May 24 2017, 1:33 PM.

Details

Summary

There are 2 parts to this patch, but I think they need to be simultaneous to avoid a regression.

  1. I'm proposing to reverse the canonicalization that moves bitwise vector ops before bitcasts. I want to move bitwise vector ops *after* bitcasts instead. That's the 1st and 3rd hunks of the patch. The motivation is that I only know of one fold that currently depends on the existing canonicalization (see next), but there are many folds that would automatically benefit from the new canonicalization. PR33138 ( https://bugs.llvm.org/show_bug.cgi?id=33138 ) shows why/how we have these patterns in IR.
  1. There's an or(and,andn) pattern that requires an adjustment in order to continue matching to 'select' because the bitcast changes position. This match is unfortunately complicated because it requires 4 logic ops with optional bitcast and sext ops. As part of making that part less ugly, I'd like to add a 'peekThroughBitcasts()' helper based on similar functions that we use in the DAG. (I can add that in a separate NFC commit if preferred.)

Test diffs:

  1. The bitcast.ll and bitcast-bigendian.ll changes show the most basic difference - bitcast comes before logic.
  2. There are also tests with no diffs in bitcast.ll that verify that we're still doing folds that were enabled by the previous canonicalization.
  3. icmp-xor-signbit.ll shows the payoff. We don't need to adjust existing icmp patterns to look through bitcasts.
  4. logical-select.ll contains several tests for the or(and,andn) --> select fold to verify that we are still handling those cases. The lone diff shows the movement of the bitcast from the new canonicalization rule.

Diff Detail

Event Timeline

spatel created this revision.May 24 2017, 1:33 PM
RKSimon accepted this revision.Jun 21 2017, 9:27 AM

LGTM. Please do the peekThroughBitcast helper as a NFC pre-commit.

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