[llvm] [Backend] Add clearSubtargetMap API for TargetMachine. (PR #112383)
weiwei chen via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 6 07:07:47 PST 2024
weiweichen wrote:
> I like the name-change to a general API rather than exposing the internal detail of `SubtargetMap`. However, with a general name like `reset()` I would expect the function to reset _all_ internal state, but I'm not sure that this PR accomplishes that for all targets. We could consider raising an unimplemented error in the base class and only adding `reset()` to the targets that you need to be a little more defensive about it.
>
> Other than that, this looks good to me.
I got other reviewer's request to make the name of the function more general although the intent is to just clear the one data structure. I'm not sure how to balance that comment with yours though I agree it makes more sense to me that `reset()` resets _all_ internal state.
I'll keep the general name of the API and remove the implementation for the targets I don't need for now to keep it as a no-op.
https://github.com/llvm/llvm-project/pull/112383
More information about the llvm-commits
mailing list