[PATCH] D62857: [RISCV] Prevent re-ordering some adds after shifts

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 18 00:41:05 PDT 2019


asb added a comment.

In D62857#1546071 <https://reviews.llvm.org/D62857#1546071>, @xbolva00 wrote:

> >> In future, DAGCombine could do the check to see whether c1 fits into an add immediate, which might simplify more targets hooks than just RISC-V.
>
> Why not do this the right way already?
>
> @lebedev.ri  @craig.topper


Now this patch has been extended to cover more cases (i.e. comparing materialisation cost rather than just identifying the isLegalAddImmediate case), it wouldn't have much effect on this backend. A patch that uses isLegalAddImmediate in the relevant DAGCombine might be a small benefit for targets that don't implement the isDesirableToCommuteWithShift hook, but I don't think it would affect the implementation here (it's not obvious to me the hook implementation should assume isLegalAddImmediate had already been checked). So if it does make sense to add, I think it's definitely a separate patch to this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62857





More information about the llvm-commits mailing list