This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Support load(gep(A, select(idx1, idx2)))) to select(load(gep(A, idx1)), load(gep(A, idx2))) transformation
Needs ReviewPublic

Authored by wmi on Nov 11 2016, 4:12 PM.

Details

Summary

It is to fix PR30950. After SimplifyCfg sinking, it will create IR as load(gep(A, select(idx1, idx2)))). Only equipped with the transformation, InstCombine can make the redundent loads hidden by select exposed to GVN.

Diff Detail

Repository
rL LLVM

Event Timeline

wmi updated this revision to Diff 77694.Nov 11 2016, 4:12 PM
wmi retitled this revision from to [InstCombine] Support load(gep(A, select(idx1, idx2)))) to select(load(gep(A, idx1)), load(gep(A, idx2))) transformation.
wmi updated this object.
wmi added reviewers: jmolloy, majnemer, mkuper.
wmi set the repository for this revision to rL LLVM.
wmi added subscribers: llvm-commits, davidxl.
majnemer added inline comments.Nov 11 2016, 4:20 PM
lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
886

auto *SI

907–908

Do not create IR if you don't think you will use it.

test/Transforms/InstCombine/load-gep-select.ll
1

I'm not sure how I feel about two different passes showing up here.

wmi added inline comments.Nov 11 2016, 7:27 PM
lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
886

Ok, will fix it.

907–908

But I only need to insert the nodes into IR after isSafeToLoadUnconditionally return true for them. Any suggestion for this case?

majnemer added inline comments.Nov 11 2016, 8:19 PM
lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
907–908

Something like:

if (isSafeToLoadUnconditionally(NewGEP1, Align, DL, GEPI) &&
    isSafeToLoadUnconditionally(NewGEP2, Align, DL, GEPI)) {
  GetElementPtrInst *NewGEP1 = cast<GetElementPtrInst>(GEPI->clone());
  GetElementPtrInst *NewGEP2 = cast<GetElementPtrInst>(GEPI->clone());
  NewGEP1->setOperand(LastIdx, SI->getOperand(1));
  NewGEP2->setOperand(LastIdx, SI->getOperand(2));
  ...
wmi added inline comments.Nov 12 2016, 10:16 AM
lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
907–908

But then NewGEP1 will be used in the if condition before it is created.