[PATCH] D116398: [SelectionDAG][RISCV] Add preferred extend of value used for PHI node

[Haocong Lu via Phabricator via llvm-commits]

Luhaocong marked 3 inline comments as done.
Luhaocong added a comment.

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

>> With this optimization, we could perform bit extend in necessary BB，instead of all predecessors of
>> return BB. At the same time, increase more machine CSE opportunities.
>
> Why would the extend be in all predecessors? Wouldn't it just be in the block with the return?
>
> Wouldn't this patch increase code size if none of the phi inputs were known to be sign extended? Your test just moves an extend to the block that produced %conv2 because %left and %right are already extended. But if they weren't wouldn't it increase the number of extends?

It‘s my mistake, what I want to express is that not all paths to return BB should do the extend. 
Sometimes this does increase code size, but I think it is valuable if you can get optimization opportunities.



================
Comment at: llvm/test/CodeGen/RISCV/prefer-extend.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 < %s | FileCheck -check-prefixes=RV32S %s
+; RUN: llc -mtriple=riscv64 < %s | FileCheck -check-prefixes=RV64S %s
----------------
jrtc27 wrote:
> craig.topper wrote:
> > What does the S mean after RV32/RV64?
> I assume it's for signext, but that has no place here
I've deleted it


================
Comment at: llvm/test/CodeGen/RISCV/prefer-extend.ll:5
+
+define dso_local signext i16 @get_pivot(i16 signext %left, i16 signext %right) {
+; RV32S-LABEL: get_pivot:
----------------
craig.topper wrote:
> Remove the dso_local
Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116398