[PATCH] D29014: [SelDag] Add FREEZE

Tue Mar 24 08:02:45 PDT 2020

spatel added inline comments.

================
Comment at: llvm/test/CodeGen/X86/freeze-legalize.ll:8-12
+; CHECK-NEXT:    movl $303174162, %eax ## imm = 0x12121212
+; CHECK-NEXT:    movl $875836468, %ecx ## imm = 0x34343434
+; CHECK-NEXT:    movl $1448498774, %edx ## imm = 0x56565656
+; CHECK-NEXT:    xorl %eax, %edx
+; CHECK-NEXT:    movl $2021161080, %eax ## imm = 0x78787878
----------------
aqjune wrote:
> spatel wrote:
> > aqjune wrote:
> > > aqjune wrote:
> > > > spatel wrote:
> > > > > Do we need to add basic simplify / constant folding for SDAG ?
> > > > > freeze ( Constant ) --> Constant
> > > > Yes, I agree it will be great. Do you want to make this patch contain the change as well?
> > > Or I can land this first, and add the simplify / constant folding for SDAG.
> > > I prefer incrementally making things because this patch itself is a big change.
> > I prefer smaller patches too. Let's make that a follow-up. 
> > 
> > Is it correct that there is very little chance that this patch will create a visible performance regression (because there should be almost no freeze instruction creation in IR yet)?
> Yes, it is.
> Currently there is only one place where freeze is introduced - https://reviews.llvm.org/D76179
> I checked that from assembly outputs of LLVM test-suite , only 3 / 5239 files are affected by this patch.
That sounds good then. But can we avoid those 3 regressions cases before or within this patch? Ideally, we don't want to knowingly regress anything.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D29014