[PATCH] D45216: [Attributes] Add IntrinsicLoweredToCall attribute.

[Matt Davis via Phabricator via llvm-commits]

mattd added a comment.

Thanks again for the reply, much appreciated!  I suppose there is nothing that really needs to change with how the isLoweredToCall predicate is presented to the rest of the codebase. At one point I wanted to move it from TTI to TargetLowering (TLI), because the predicate is about lowering semantics, and it seemed to make sense that isLoweredToCall could live there too.  TargetLowering doesn't rely on the CRTP like TTI, so if we did move the predicate, we'd probably make it virtual.  But for  now, I don't see an advantage in moving it; however, thanks for pointing out the template magic, CRTP, and I'm now convinced that moving the predicate isn't really necessary, in other words it can still be overriden by a target implementation based on TTI.

Regarding the attribute:  I was a bit biased towards implementing this functionality as an attribute, given discussion on the previous review that brought the limitation of isLoweredToCall to my attention: https://reviews.llvm.org/D41104.  However, you put forward a good point regarding 'trunc.' and a handful of other intrinsics.

Let me step back for a minute and more generally address what I am trying to solve, in the norecurse case, and for potentially other cases as well.  What I really want to know is if an intrinsic is just metadata or not.  If the intrinsic is only an "information passer" used for communicating with other parts of the compiler (e.g., dbg.value, lifetime.start, etc).  While that will not solve isLoweredToCall, I wouldn't necessarily need to use isLoweredToCall in the cases I am interested in.  I believe this will solve the issue with norecurse making decisions on 'metadata' intrinsics (llvm.dbg.).  If adding an attribute to denote  that an intrinsic will only be used as metadata  sounds like a reasonable change, then I'd be happy to rename the changes in this patch to something more appropriate, such as: "Attribute::MetadataIntrinsic", and then update the obvious metadata specific intrinsics in Intrinsics.td.


https://reviews.llvm.org/D45216