[clang] [clang-tools-extra] Remove `StringLiteral` in favor of `StringRef` (PR #122366)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

> > > Just that the last bit of performance
> > 
> > 
> > Maybe we're talking about different situations; I may have missed some context in the discussion of this PR. I didn't see this as being about the last bit of performance. It sounded like this was going to knowingly introduce a performance regression in an popular configuration (RelWithDebInfo) for developing Clang because it made for cleaner code. That's not a good tradeoff IMO. That said, different contexts might find that tradeoff worthwhile. I just think we need to be careful -- developing LLVM is already frustrating to some folks due to our resource requirements and I have anecdotal evidence that it has prevented us from gaining new contributors, so I worry about steps which make that situation worse. If it benefits end users, that's one thing. But this sounded like it primarily only benefits ourselves, which is a harder sell to me.
> 
> Looks like we were talking about whether MSVC constant evaluated strlen & whether we should workaround it not doing that.
> 
> It's a bit of an abstract discussion - since it seems the premise isn't true (MSVC did manage to compile the strlen to a constant in at least a simple example linked on godbolt earlier).
> 
> But even if it were true - I think there's some line where we'd say the extra performance on MSVC isn't worth the extra complexity to the codebase. I guess we don't have any concrete numbers on the abstract question (what such a missed optimization would cost, since it doesn't appear to exist in reality) - but my balance for that is fairly strongly tipped in favor of avoiding that complexity and for folks building LLVM with MSVC to see some perf cost for it. How much perf cost is too much? How much code complexity is too much? Don't know.

Yeah, I suspect reasonable people have different answers for that, too. I think we're not too far off in what we're hoping for though, which is basically for the benefits to outweigh the costs.

> > > I'd generally recommend folks use clang for developing with LLVM anyway
> > 
> > 
> > FWIW, that's not what I recommend. I recommend folks use whatever (supported) development environment meets their needs because that gets us the most real-world experience with things like how Clang compares to other toolchains in terms of features and behavior. I use Visual Studio as my daily driver and that has led to a number of improvements in Clang over the years, both in terms of language extension compatibility and in terms of other compiler features. (Not to suggest you are wrong to recommend using Clang! Just that it's a slightly different way of looking at how we get contributions.)
> 
> I'm mostly OK with that too (though you can use clang in VS, right?) but hope we don't end up complicating the codebase substantially to improve performance in that situation. That it works, yes, important, that it performs usably, yes, important.

You can use Clang in Visual Studio, but I specifically want to use cl so that I see what new diagnostics they're issuing, what new weird codegen starts happening, that sort of thing. Basically, it helps with test coverage in various ways. But I think the ship has sailed about not complicating the codebase because of toolchain-specific performance behavior. One prime example is with bit-field packing. In Clang, we use `unsigned` for all bit-fields because cl doesn't pack adjacent fields together unless they have the same type. So an `int` and a `bool` and an enumeration as the underlying type for adjacent bit-fields causes problems. This makes the debugging experience worse for everyone, but to counter that, we came up with the `LLVM_PREFERRED_TYPE` macro (which hides a Clang-specific attribute we implemented for this case) to try to recapture some of that debugging aid for some folks. That said, I think we agree that we don't want to do that sort of thing too often -- it just happens that the tradeoffs in this case are worth the pain.

> Appreciate the perspectives, though - bit surprised/good to understand the diversity of perspectives.

100% agreed, this is a great discussion!

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