Page MenuHomePhabricator

GlobalISel: Merge G_PTR_MASK with llvm.ptrmask intrinsic

Authored by arsenm on May 15 2020, 6:23 PM.



Confusingly, these were unrelated and had different semantics. The
G_PTR_MASK instruction predates the llvm.ptrmask intrinsic, but has a
different format. G_PTR_MASK only allows clearing the low bits of a
pointer, and only a constant number of bits. The ptrmask intrinsic
allows an arbitrary mask. Replace G_PTR_MASK to match the intrinsic.

Only selects the cases that look like the old instruction. More work
is needed to select the general case. Also new legalization code is
still needed to deal with the case where the incoming mask size does
not match the pointer size, which has a specified behavior in the

Diff Detail

Event Timeline

arsenm created this revision.May 15 2020, 6:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2020, 6:23 PM
gargaroff accepted this revision.May 17 2020, 11:11 PM

LGTM with a nit. Thanks for unifying the semantics!


Should the verifier report where the type mismatch is? It already reports if the destination of the mask is incorrect. Shouldn't it do the same for the source?

This revision is now accepted and ready to land.May 17 2020, 11:11 PM
arsenm marked an inline comment as done.May 18 2020, 3:02 PM
arsenm added inline comments.

Generally the verifier seems to just try every single check independently and produce every error it can, even if it's redundant

dsanders added inline comments.May 19 2020, 7:14 PM

Did you mean to drop the underscore? It's inconsistent with G_PTR_ADD now


Could you add these requirements to GenericOpcodes.rst (and correct the semantics in that description too)?

arsenm marked an inline comment as done.May 19 2020, 7:22 PM
arsenm added inline comments.

Yes. It's now consistent with the IR intrinsic

arsenm closed this revision.May 26 2020, 8:48 AM
arsenm marked 2 inline comments as done.

Really I'd rather rename the IR intrinsic, but it's probably not worth the hassle