[PATCH] D29014: [SelDag][MIR] Add FREEZE

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/Target/Target.td:1067
+  let hasNoSchedulingInfo = 1;
+  let isNotDuplicable = 1;
+}
----------------
aqjune wrote:
> arsenm wrote:
> > arsenm wrote:
> > > aqjune wrote:
> > > > arsenm wrote:
> > > > > Why isNotDuplicable? I don't think this is the best maintained instruction property
> > > > It is because different freeze defs can yield different values. For example, consider following transformation:
> > > > ```
> > > > %undef = IMPLICIT_DEF
> > > > %x = FREEZE %undef
> > > > use(%x)
> > > > use(%x) // these two uses should see the same freezed value
> > > > ->
> > > > %undef = IMPLICIT_DEF
> > > > %x = FREEZE %undef
> > > > use(%x)
> > > > %x' = FREEZE %undef
> > > > use(%x') // It is possible that %x and %x' are assigned differently freezed values.
> > > > ```
> > > > This transformation is incorrect, because use()s after the transformation can see different values.
> > > > To prevent this class of optimizations, isNotDuplicable is set to 1.
> > > But these both are using the same undef vreg? The second freeze is still using the original undef, so this should be fine?
> > To clarify, I think the property you are really looking for is not the property given to you by isNotDuplicable. What you really care about is the input operand not using a new instance of undef. The "nonduplicability" is a function of the input value and not the instruction itself
> The second use of the undef register can be assigned a different physical register at (perhaps) register allocation, after seeing that its definition is IMPLICIT_DEF.
> ```
> %undef = IMPLICIT_DEF
> %x = FREEZE %undef
> use(%x)
> %x' = FREEZE %undef
> use(%x')
> =>
> %x = FREEZE $eax
> use(%x)
> %x' = FREEZE $ebx
> use(%x')
> ```
> This transformation can happen if there is a function call between them. `test/CodeGen/X86/freeze-call.ll` checks that it never happens.
> 
I think handling this correctly requires changing to how IMPLICIT_DEF  is handled in register allocation. I don't think noduplicate is really going to model this constraint sufficiently


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

https://reviews.llvm.org/D29014