[PATCH] Update InstCombine to transform aggregate loads into scalar loads.

Amaury SECHET deadalnix+llvmreview at gmail.com
Fri Apr 10 17:45:25 PDT 2015


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:316
@@ -316,4 +315,3 @@
 /// point the \c InstCombiner currently is using.
-static LoadInst *combineLoadToNewType(InstCombiner &IC, LoadInst &LI, Type *NewTy) {
-  Value *Ptr = LI.getPointerOperand();
-  unsigned AS = LI.getPointerAddressSpace();
+static LoadInst *combineLoadToNewValue(InstCombiner &IC, LoadInst &LI, Value *V,
+                                       Type *NewTy, const Twine& Suffix = "") {
----------------
reames wrote:
> The naming here is confusing to the point where I'm having a hard time telling what you intend.  If I'm reading this right, by Value you mean address loaded from?  You're changing both the type loaded and the pointer operand in one go?
> 
> If so, I might suggest the following:
> - pass both the pointer operand and new type explicitly.  This will be necessary for the new unified pointer type work and helps readability now.
> - Don't use the name V.  Call it NewPointerOperand or something similiar.  NewAddr would also be fine.
> - Clarify the comment about what the function does.  
> 
> You might also consider changing the type loaded and the pointer operand in two distinct steps.
> NewLI = combineLoadToType(*this, OldLI, ElementType);
> NewLI->setPointerOperand(ElementPtr);
> 
> The intermediate state (load type X from a pointer of type Y using a bitcast) is entirely legal.  
> 
> 
setPointerOperand does not exists, but that is no big deal, a bitcast is just as valid here, so let's do that, it can make the diff much simpler.

http://reviews.llvm.org/D8339

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list