[PATCH] D93818: [LangRef] Update shufflevector's semantics to return poison if the mask is undef

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune added inline comments.


================
Comment at: llvm/docs/LangRef.rst:9385
+using ``undef`` for ``val`` is not recommended because it prohibits
+optimizations to '``shufflevector``' with ``poison`` masks.
+
----------------
efriedma wrote:
> This seems like a weird thing to say.  It's not really a semantic rule, just a suggestion for frontend and/or optimizations.  Not sure it's really worth saying anything here at all.  I mean, if we just want to give general advice to frontend/optimizations authors, it would just be "prefer poison over undef where possible", which isn't really specific to insertelement.  Maybe we could just add an example to the "example" block.
This is Arguments block, so I think talking about it is okay.
I think adding an example is a good idea, will put one to the example block.



================
Comment at: llvm/docs/LangRef.rst:9445
+the shuffle mask selects a poison element from one of the input
+vectors, the resulting element is poison. A poison element
+in the mask vector specifies that the resulting element is poison.
----------------
efriedma wrote:
> Maybe also explicitly note that if we select an undef element from an input vector, the resulting element is undef?
> 
> Any thoughts on whether we need to do anything for bitcode autoupgrade?  I mean, it's unlikely to be a practical issue, but we could theoretically break the semantics of old IR.  We could freeze the result to preserve the semantics of old bitcode.
> Maybe also explicitly note that if we select an undef element from an input vector, the resulting element is under?

+1

> 
> Any thoughts on whether we need to do anything for bitcode autoupgrade?  I mean, it's unlikely to be a practical issue, but we could theoretically break the semantics of old IR.  We could freeze the result to preserve the semantics of old bitcode.

This sounds legit, but it will introduce a lot of freeze instructions with extractelement/insertelement combined.
I'd like to hear other reviewer's opinion as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93818