[PATCH] D16275: [opaque pointer types] [NFC] GEP: replace get(Pointer)ElementType uses with get{Source, Result}ElementType.

[David Blaikie via llvm-commits]

On Tue, Jan 19, 2016 at 3:34 PM, Eduard Burtescu via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> eddyb added inline comments.
>
> ================
> Comment at: llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp:516
> @@ -515,2 +515,3 @@
>      } else {
> -      Ty = cast<SequentialType>(Ty)->getElementType();
> +      if (Ty->isPointerTy()) {
> +        // The only pointer type is for the very first index,
> ----------------
> dblaikie wrote:
> > This seems like a slightly indirect test of the property you describe in
> the comment.
> >
> > Do you think it might be nicer to check if the iterator is ==
> I->op_begin() + 1? Or to add a flag of some kind before the loop for this
> first case? Or perhaps to use a gep type iterator in parallel with the
> operands? (or an assert for any of these conditions)
> >
> > Not necessary, just throwing some ideas around in case any of them seem
> like an improvement.
> I think I would use `gep_type_iterator`, but I would have to rewrite each
> of these loops (and maybe make `gep_type_iterator` nicer to use, with C++11
> range-for sugar perhaps).
>
> Although, now that I've gotten myself set with a fast rebase/rebuild
> cycle, it shouldn't be such an ordeal anymore, I might just try it out.
>

Yeah, by no means a hard requirement - was just wanting to consider a bit
if there's something nicer to avoid the repetition. Don't exhaust yourself
on it unless it's of interest/looks likely to yield results.

- Dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160119/905b3461/attachment.html>