[PATCH] D13427: RFC: faster isa<IntrinsicInst> (bugged tests?)

[Philip Reames via llvm-commits]

My opinion is that a name does not an intrinsic make.  Unless someone 
has an existing use case which requires the ability to implicitly add 
intrinsics, I would suggest we do the following:
- Add a rule to the verifier that explicitly requires any function with 
an @llvm prefix to have a known intrinsic ID.  It should be an error to 
use an @llvm. prefix for anything which is not a well known intrinsic.
- All of the tests which are using an unknown intrinsic are modified to 
use @llvm.do_nothing which exists exactly for this purpose.

Once those two steps are done, making isIntrinsic check the intrinsic ID 
seems entirely reasonable to me.

p.s. Is there any reason that recalculateIntrinsicID needs to be 
public?  I don't see an obvious one.

p.p.s. I would also be interested in seeing the code generated for the 
current comparison.  We should be able to emit fast code for that, but 
it's an entirely separate issue from what we do in terms of defining 
intrinsics.  Doing a string check just isn't needed here.

Philip


On 10/05/2015 10:24 PM, hfinkel at anl.gov wrote:
> hfinkel added a subscriber: hfinkel.
> hfinkel added a comment.
>
> I recommend that you un-abandon this, and I'd like to hear Philip's opinion here. As I recall, he's spent some time looking at the time spent in string-matching related to intrinsics.
>
> Could you please post this with full context, however?
>
>
> Repository:
>    rL LLVM
>
> http://reviews.llvm.org/D13427
>
>
>