[PATCH] D18732: [SystemZ] Support LRVH and STRVH opcodes

Ulrich Weigand via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 4 05:37:19 PDT 2016


uweigand added a comment.

In http://reviews.llvm.org/D18732#391033, @bryanpkc wrote:

> In http://reviews.llvm.org/D18732#391023, @uweigand wrote:
>
> > Do those DAG patterns actually not match?  If so, it might be better to not provide any.  (Or of course even better, provide ones that actually work.)   Or do they in fact match, and you just didn't verify that?  If so, it would be good update README.txt accordingly and add a CodeGen test to verify correct code generation.
>
>
> I think it would be better to make the codegen emit LRVH and STRVH properly. I will verify that the patterns match correctly and add some test cases. Was there a reason why README.txt explicitly stated that LRVH and STRVH were not supported?


Well, it's a simple fact that in the current code they aren't supported.  The more interesting question is, why didn't Richard just add them right away when he added the others?  Unfortunately I don't recall the reason for that, it's been this way since the beginning.

One potential hick-up may be that i16 is not a legal type in the back-end, which means that the DAG may not actually contain a plain "load" node, but some form of an extending load.  This would mean the matching DAG pattern would have to be updated accordingly.


http://reviews.llvm.org/D18732





More information about the llvm-commits mailing list