[PATCH] D59065: Add ptrmask intrinsic

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 199460.
fhahn added a comment.

In D59065#1500576 <https://reviews.llvm.org/D59065#1500576>, @jdoerfert wrote:

> In D59065#1497965 <https://reviews.llvm.org/D59065#1497965>, @fhahn wrote:
>
> > In D59065#1497227 <https://reviews.llvm.org/D59065#1497227>, @jdoerfert wrote:
> >
> > > What happens if I use an integer type bigger than the native pointer width? Or smaller?
> >
> >
> > Initially I wanted to restrict the mask type to the native pointer width, but couldn't find how to do that in Intrinsics.td. On second thought, this might be more restrictive than necessary. I've adjusted the wording to make it clear that we zero-extend or truncate the mask if the bit width does not match the pointer size of the target. What do you think?
>
>
> The wording makes it clear now (what is supposed to happen at least).


Thanks!

> Last points from my side:
> 
> - What happened to the poison result sentence. I think we need that to ensure the "based on the same ..." part. Otherwise, we can butcher the pointer with the mask and end up with a "valid pointer" into a different object. I might be wrong here, just want to make sure we have thought of it.

My understanding from Hal's comments was that the behavior should be clear from the pre-existing semantics, but I might have missed something?

The case where we fail to clear bits that must be zero should be clearly handled by the existing semantics. The case where we might clear meaningful bits and/or end up in a situation where we have multiple ptrmask calls for the same pointer and different masks should be OK, as that is what is asked for in the IR. If this creates invalid pointers, the existing semantics handle that case too.

> - Can we add the `speculatable` attribute? Executing `ptrmask` should never cause UB (as we agreed on) so we need `speculatable` to express that, right?

Done, that should be fine. UB can only occur when using the pointer.


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

https://reviews.llvm.org/D59065

Files:
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/test/CodeGen/AArch64/lower-ptrmask.ll
  llvm/test/CodeGen/X86/lower-ptrmask.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59065.199460.patch
Type: text/x-patch
Size: 5208 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190514/e48717fa/attachment.bin>