[llvm-commits] Updated pointer-vector patch

[Nick Lewycky]

Rotem, Nadav wrote:
>
>
> Hi Nick,
>
> Thank you for the detailed code review. I went over it carefully and
> made the corrections you requested. I added all of the testcases that
> you mentioned, as well as a few other tests. I added basic unit tests
> for the functions you requested. I fixed several optimizations which
> assumed that the GEP pointer operand is a pointer (go figure). I fixed
> getPointerOperandType, and added Type::getNumElements. I checked the cpp
> backend and it works. I don’t plan to add vector-load as part of this
> patch because I think that it will require lots of codegen work.

Yes, thanks. It would also require re-verifying all of llvm for places 
that assume loads can only operate on pointers. That is going to be a 
very time-consuming cleanup.

> GetPointerBaseWithConstantOffset does not crash, but also does not
> implement the vector offset functionality. I prefer to improve
> vector-gep optimizations in future patches. There are a few users who
> are looking forward to start using this feature, and I am sure that they
> will help in ironing out the last few bugs.

Yep! I don't want to spend time working on optimizations for the new 
type until after it's landed, but I do want to make sure that the 
existing code will work with it.


+  PointerType *PtrTy = dyn_cast<PointerType>(Ops[0]->getType());
+  // The GEP pointer operand is not a pointer, its a vector of pointers.
+  if (!PtrTy)
+    return 0;

Typo: "its" -> "it's".

LGTM, please commit!

Nick