[PATCH] D39758: CodeGen: Fix pointer info in SplitVecOp_EXTRACT_VECTOR_ELT

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1787
   return DAG.getExtLoad(ISD::EXTLOAD, dl, N->getValueType(0), Store, StackPtr,
-                        MachinePointerInfo(), EltVT);
+                        MachinePointerInfo(UndefValue::get(Type::getInt32PtrTy(
+                            *DAG.getContext(), PtrInfo.getAddrSpace()))),
----------------
yaxunl wrote:
> efriedma wrote:
> > You can't use "UndefValue" here; a non-null "Value*" has to be something alias analysis will understand properly.
> > 
> > If you want to actually get this right, you need to use MachinePointerInfo::getFixedStack().  (CreateStackTemporary doesn't explicitly return the FrameIndex, but you can extract it from the returned FrameIndexSDNode.)  Or I guess we could add some way to represent an unknown pointer in a specific address-space, but that's probably not any simpler.
> getFixedStack can only be used with frame index with a constant offset, but here the offset is not constant. Here we only need to carry the information about address space. Therefore undef with a specific address space. The backend won't perform any optimization based on that and only uses the address space information.
> getFixedStack can only be used with frame index with a constant offset, but here the offset is not constant.

Okay.  You might need some other PseudoSourceValue, then.

> Therefore undef with a specific address space.

undef means "this access doesn't alias anything".  See BasicAAResult::aliasCheck.

> The backend won't perform any optimization based on that

Among other things, we use MachinePointerInfo for scheduling... so if you get the info wrong, we could move the load before the store.


https://reviews.llvm.org/D39758