[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

Mon Oct 18 13:36:43 PDT 2021

craig.topper added a comment.

In D105168#3071330 <https://reviews.llvm.org/D105168#3071330>, @jrtc27 wrote:

> In D105168#3071321 <https://reviews.llvm.org/D105168#3071321>, @craig.topper wrote:
>
>> In D105168#3071249 <https://reviews.llvm.org/D105168#3071249>, @jrtc27 wrote:
>>
>>> Two options come to mind if we really need to be outputting a StringRef list:
>>>
>>> 1. (the simpler option) Pass in a Twine -> `const char *` lambda that the caller hooks up to Args.MakeArgString
>>
>> Good idea. I'll post a patch for that shortly.
>>
>>> 2. (probably the nicer option) Invent our own MakeArgString that allocates from storage owned by RISCVISAInfo itself; lots of ways you can do that
>>
>> I don't think the RISCVISAInfo objects lives long enough for that. It only lives for the duration of the function that calls toFeatures, but the Features vector is returned from that function.
>
> Hm, I see. We could also just have a global cache of heap-allocated copies of these strings, I guess; after all it's not all that different from the tables of strings we already have, just lazily built up at run time (can even store them in a lazily-initialised field of each RISCVExtensionInfo). Or we just stick to the slightly-ugly lambda approach, unless anyone else has bright ideas for a cleaner interface and implementation.

Lambda approach here https://reviews.llvm.org/D112032

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105168/new/

https://reviews.llvm.org/D105168