[PATCH] D11363: Allow merging of immediates within a basic block for code size savings and reduced footprint.

[Michael Kuperstein]

mkuper added a comment.

In http://reviews.llvm.org/D11363#214332, @qcolombet wrote:

> Hi Michael,
>
> I am not sure I understand this part:
>
> > We sill still pull the constant into the instruction during selection.
>
>
> I thought ConstantHoisting was marking the constants as opaque so that this cannot happen. What am I missing?


Not opaque enough.
It's opaque in terms of cross-basic-block propagation, but not within a block.
That is, for this:

  define i32 @test1(i32 %a) {
    %const = bitcast i32 42 to i32 
    br label %next
  next:
    %1 = add i32 %a, %const
    %2 = add i32 %1, %const
    ret i32 %2
  }
  
  define i32 @test2(i32 %a) {
    %const = bitcast i32 42 to i32 
    %1 = add i32 %a, %const
    %2 = add i32 %1, %const
    ret i32 %2
  }

We get:

  test1:                                  # @test1
          movl    4(%esp), %eax
          movl    $42, %ecx
          addl    %ecx, %eax
          addl    %ecx, %eax
          retl
  
  test2:                                  # @test2
          movl    4(%esp), %eax
          addl    $42, %eax
          addl    $42, %eax
          retl



> 

> 

> >   [...] but the instruction selection changes are needed regardless of that.

> 

> 

> Which ones are needed?


The ones in this review - handling basic-block-local constants in a smarter way.
One possible alternative is getting ConstantHoisting to always hoist the constants into a new BB, but that seems fairly ugly to me.


http://reviews.llvm.org/D11363