[PATCH] D132978: [IRTranslator] Using ZExt for extractelement indices.

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/docs/LangRef.rst:9694-9695
 the position from which to extract the element. The index may be a
-variable of any integer type.
+variable of any integer type, and will be zero extended or truncated 
+depending on the target.
 
----------------
Peter wrote:
> foad wrote:
> > From a semantics point of view, I think all we need to say is that the value is "treated as unsigned" or words to that effect. I don't think we should mention extending here (and definitely not truncating which sounds like it could change the value!).
> https://llvm.godbolt.org/z/Mszb7EvPe
> 
> The (unfortunate) fact that we are using `zextOrTrunc` already implies that we can change the value given a large index, even thou no one is doing it.
> I see two ways out of this:
> 
> 1. Explain in detail in the doc "when" truncation will happen or
> 2. Further change the doc so the index can only be i64, which would involve even more code change.
The document should describe the intended semantics of the IR, not the details of the current GlobalISel implementation.

(You're correct that GlobalISel does odd things if the index type is wider than 64 bits, but I don't think it's really *wrong*. The only effect it can have is to change an out-of-range index into an in-range index. And if the index was out of range in the first place then the result was undefined anyway (or poison) so it doesn't really matter what GlobalISel does with it.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132978/new/

https://reviews.llvm.org/D132978