[PATCH] D26174: DAG: Avoid OOB when legalizing vector indexing

[Eli Friedman via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3607
+  if (isa<ConstantSDNode>(Idx))
+    return Idx;
+
----------------
arsenm wrote:
> efriedma wrote:
> > arsenm wrote:
> > > efriedma wrote:
> > > > I'm pretty sure this special-case isn't safe; just because it's a constant doesn't mean it's in range.
> > > I was wondering about this, however I think it's OK. getNode for EXTRACT_VECTOR_ELT turns this into undef for out of bounds. For INSERT_VECTOR_ELT it seems to not, but I also haven't been able to come up  with a testcase where this gets hit
> > If getNode() is actually enforcing the restriction, fine... otherwise, we're likely to run into issues which are extremely difficult to reproduce because the constant is getting produced by splitting an integer in legalization or something like that.
> Should I fix getNode not doing this for insert_vector_elt to match instead? It seems broken to me that it would for one but not the other
Actually, thinking about it a bit more, I would prefer not to leave this issue around as a trap if we start using getVectorElementPointer outside of insert/extract legalization, where we might not enforce a "constants are in range" rule.  The check isn't doing anything useful anyway.

(Of course, fixing getNode would be fine.)


https://reviews.llvm.org/D26174