[PATCH] D34576: [ConstantHoisting] Avoid hoisting constants in GEPs that index into a struct type.

[Leo Li via Phabricator via llvm-commits]

aoli added a comment.

@efriedma  thank you for pointing out!

I just did some tests and I found there still some potential bugs in constant hoisting.

For insertvalue

  define void @test1(%T %P) {                                                     
    %A = insertvalue %T %P, i32 256, 256                                          
    %B = insertvalue %T %P, i32 256, 256                                          
    %C = insertvalue %T %P, i32 256, 256                                          
    ret void                                                                      
  }  

will be optimized to

  define void @test1(%T %P) {
    %const = bitcast i32 256 to i32
    %A = insertvalue %T %P, i32 %const, 256
    %B = insertvalue %T %P, i32 %const, 256
    %C = insertvalue %T %P, i32 %const, 256
    ret void
  }

which is wrong (? I'm not very sure here but based on the comments in llvm::canReplaceOperandWithVariable: )

It may good for us to use llvm::canReplaceOperandWithVariable: to maintain the constancy.


https://reviews.llvm.org/D34576