[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