[PATCH] D123264: [RISCV] Pre-RA expand pseudos pass

Luís Marques via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 29 16:55:04 PDT 2022


luismarques added a comment.

In D123264#3688467 <https://reviews.llvm.org/D123264#3688467>, @craig.topper wrote:

> In D123264#3682868 <https://reviews.llvm.org/D123264#3682868>, @jrtc27 wrote:
>
>> LA should follow LLA; at least the LA-as-LLA expansion should follow the same behaviour as they are supposed to be equivalent for non-fPIC.
>
> Does the compiler ever emit an LA-as-LLA? The branch in RISCVExpandPseudoInst is untested according to the coverage report https://lab.llvm.org/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp.html#L217



In D123264#3688467 <https://reviews.llvm.org/D123264#3688467>, @craig.topper wrote:

> In D123264#3682868 <https://reviews.llvm.org/D123264#3682868>, @jrtc27 wrote:
>
>> LA should follow LLA; at least the LA-as-LLA expansion should follow the same behaviour as they are supposed to be equivalent for non-fPIC.
>
> Does the compiler ever emit an LA-as-LLA? The branch in RISCVExpandPseudoInst is untested according to the coverage report https://lab.llvm.org/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp.html#L217

I think that branch was already dead when it was originally added: https://reviews.llvm.org/D55303
`RISCV::PseudoLA` was only emitted inside an `if (isPositionIndependent())`, and the dead branch is the negation of that condition.
Should we remove the dead branch in a separate patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123264



More information about the llvm-commits mailing list