[PATCH] D78545: [RISCV] Make CanLowerReturn protected for downstream maintenance
Jessica Clarke via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 12 00:30:17 PDT 2020
jrtc27 added a comment.
In D78545#2030910 <https://reviews.llvm.org/D78545#2030910>, @Jim wrote:
> In D78545#2030863 <https://reviews.llvm.org/D78545#2030863>, @jrtc27 wrote:
>
> > I am still very confused why this is necessary. `CanLowerReturn` barely does anything other have a loop to call other functions that actually do useful work, it seems like the wrong place to hook into things. Your "explanation" is merely just "we want this because we want this". Could you please shed some light on how _exactly_ this is useful? As it stands I am none the wiser.
>
>
> In `CanLowerReturn`, it calls `CC_RISCV` in a loop to check if the type can be returned directly. Actually, the change should be in `CC_RISCV` if we want a specific type returned indirectly.
> But this change is maintained hard for downstream while we upgrade or merge the code from trunk. Let `CanLowerReturn` public/protected for overridden and reusing by the derived class without touching `CC_RISCV`.
Yes, exactly, your code belongs in `CC_RISCV`, not `CanLowerReturn`. That's what we do for our fork, and that in and of itself rarely causes merge conflicts, because that code rarely needs to change upstream.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78545/new/
https://reviews.llvm.org/D78545
More information about the llvm-commits
mailing list