[llvm] [TableGen] Rework `EmitIntrinsicToBuiltinMap` (PR #104681)

[Rahul Joshi via llvm-commits]

jurahul wrote:

> > > > > Thanks @arsenm. I realized though that change in the `BuiltinEntry` struct is actually a space regression. We have one
> > > > > ,,,
> > > > > @Artem-B ^^
> > > 
> > > 
> > > I doubt that the space used for builtins info makes any practical difference. If it does not affect or improves performance, I'm fine with it.
> > 
> > 
> > In the sense, fine with using the old form, or fine with the extra bloat for clarity (i.e., switch to using StringRef)?
> 
> Fine as in "there are more important factors. do what makes sense". My understanding is that the goal is to improve performance. We also want to maintain or improve readability. Incremental increase in space use is a reasonable trade-off, if it buys us something useful.
> 
> > I agree that this space is not going to make a difference, but migrating to StringRef does not help perf as well,
> > but does make the code easier to understand (in case someone decides to read this generated code). Please let me know your preference.
> 
> Does it make any measurable performance difference in practice? If not, readability wins.
> 
> That said, in this case both the likely performance impact and the readability improvement are cosmetic, so I'll leave it up to you. My follow up comment was mostly because I was not sure whether the change was omitted intentionally or just slipped through the cracks.

Yes, AFAICT it does not make a performance difference in the benchmark I added. No, I did upload a version with the change you had suggested, but then reverted to the old code to address the space bloat.

https://github.com/llvm/llvm-project/pull/104681