AMD's optimization manual for bulldozer only shows a 2 cycle latency. I'm not sure why Agner reports 20 unless there's some bad case for some particular input that isn't documented. A single uop taking 20 cycles sounds very strange and must be serializing the machine. I would only expect divide/sqrt to be that high from a single uop. Maybe someone can run llvm-exegesis and one of those AMD CPUs

Add constant argument range checking to SemaChecking

In D104853#2838943, @sepavloff wrote:

In D104853#2838797, @craig.topper wrote:

FXAM appears to be two uops where FTST is one on modern Intel CPUs based on Agner Fog's data. Agner's data for some AMD CPUs shows ~20 cycles of latency.

Could tuning scheduling for this instruction be subsequent work?

lgtm

FXAM appears to be two uops where FTST is one on modern Intel CPUs based on Agner Fog's data. Agner's data for some AMD CPUs shows ~20 cycles of latency.

Doesn't gcc also fold isnan to false under fast math? If we diverge here that means your code would only work correctly with clang.

Honor zext function attributes over a preference for sext.

We may want to update the code in X86ISelLowering getAVX2GatherNode and getGatherNode to replace freeze+poison on Src with a zero vector. We already do this when the Src is undef.

Reword a comment slightly

clang-format

In D104790#2836253, @aqjune wrote:

I couldn't find end-to-end tests for checking assembly generation.
To check whether this is working ok, which tests should I write and how would it look like?

LGTM

LGTM

LGTM

In D104308#2830379, @frasercrmck wrote:

In D104308#2830045, @simoll wrote:

Disabling lanes is really what makes the difference between these and the regular reduction intrinsics.
There is also the corner case that all lanes are disabled and i am unsure what the return value should be then. Any thoughts on that?

Agreed. I'll update the docs accordingly.

We have a couple of options for what happens when all lanes are disabled. For starters, it follows logically from the definition of the expansion into reduce(select %mask, %v, %neutral) that we just return the neutral element. So that's the "easiest" definition in that sense.

We could return undef too which would be closer to how the other VP intrinsics work.

As for other options, it wouldn't make sense to me to specify that we return any of the vector elements (e.g. v[0]) as they're not conceptually active. And I think poison wouldn't make any sense. Are those the only realistic options?

In terms of hardware (which is orthogonal but may help guide us), RVV always takes a start value so we'd just return the neutral element; I admit I might be surreptitiously led by that. The lowering for returning the "neutral element" may be complicated on some targets, involving fetching the active length and perhaps doing some kind of scalar select. How would VE work?

In D104440#2830256, @LuoYuanke wrote:

I'm not sure. To do it in the stackifier you need to do it any time the undef flag is present regardless of whether StackTop is 0. You instead would need to check whether the register is already present in the stack and only insert if it isn't. But there may be some complications with removing it from the stack later. I think removing things from the stack is based on kill flags, but I don't know if the undef would have a kill flag. So I guess you'd have to remember you inserted it and immediately remove it after the instruction? You would need to do this for any FP instruction not just ArgFPRW.

It make sense. Fixing in ISel is easier. Do you mind if I create another patch in phabricator based on your patch, or you prefer to finishing the patch by yourself?

In D104440#2829512, @LuoYuanke wrote:

Does this fix your test

diff --git a/llvm/lib/Target/X86/X86FastISel.cpp b/llvm/lib/Target/X86/X86FastISel.cpp
index 44670a9..3e5d45b 100644
--- a/llvm/lib/Target/X86/X86FastISel.cpp
+++ b/llvm/lib/Target/X86/X86FastISel.cpp
@@ -3842,6 +3842,30 @@ unsigned X86FastISel::fastMaterializeConstant(const Constant *C) {
     return X86MaterializeFP(CFP, VT);
   else if (const GlobalValue *GV = dyn_cast<GlobalValue>(C))
     return X86MaterializeGV(GV, VT);
+  else if (isa<UndefValue>(C)) {
+    unsigned Opc = 0;
+    switch (VT.SimpleTy) {
+    default:
+      break;
+    case MVT::f32:
+      if (!X86ScalarSSEf32)
+        Opc = X86::LD_Fp032;
+      break;
+    case MVT::f64:
+      if (!X86ScalarSSEf64)
+        Opc = X86::LD_Fp064;
+      break;
+    case MVT::f80:
+      Opc = X86::LD_Fp080;
+      break;
+    }
+
+    if (Opc) {
+      Register ResultReg = createResultReg(TLI.getRegClassFor(VT));
+      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(Opc), ResultReg);
+      return ResultReg;
+    }
+  }
 
   return 0;
 }

Yes. It can fix. Thank you! One question, is there any other consideration to prevent undef float value in MIR? Otherwise stackify pass can support undef value by insert fld0 instruction, so that all ISel passes don't have to handle it specially.

In D104440#2829260, @LuoYuanke wrote:

I think because I use the

In D104440#2829221, @craig.topper wrote:

In D104440#2829220, @LuoYuanke wrote:

With -O0, the small case can also generate "IMPLICIT_DEF" and "CHS_Fp80". I think we are near to the root cause. Stay tuned.

ProcessImplicitDefs doesn’t run with O0 and FP stackifier has special code for IMPLICIT_DEF.

This is because I set opt-bisect-limit=67436 in my command line. When CurBisectNum expired, "DAG to DAG" pass lower its opt level to O0. However "processimpdefs" and "X86 FP Stackifier" is not stopped due to the CurBisectNum expiration. So undefined fp0 is generated.

if (OptLevel != CodeGenOpt::None && skipFunction(Fn))
  NewOptLevel = CodeGenOpt::None;
OptLevelChanger OLC(*this, NewOptLevel);

In D104440#2829220, @LuoYuanke wrote:

With -O0, the small case can also generate "IMPLICIT_DEF" and "CHS_Fp80". I think we are near to the root cause. Stay tuned.

In D104440#2829198, @LuoYuanke wrote:

I don’t know what llvm-extract —recursive does. I’ve only used -func to extract a single function I knew caused a compiler crash.

Converting IMPLICIT_DEF to undef flag is correct.

IMPLICIT_DEF can get created from ISD::UNDEF but as far I could see ISD::UNDEF for fp80 is supposed to Expand to ConstantFP 0.

I also think fneg of undef should be folded by getNode when it is created in SelectionDAGBuilder. Is this going through fast isel or something?

In my small test case that is extracted by llvm-extract, the "fneg contract x86_fp80 undef" is lowered to "LD_Fp080". But in the big case which cause crash, it is lowered to "IMPLICIT_DEF" and "CHS_Fp80".

76:                                               ; preds = %74
  %77 = call contract x86_fp80 @_ZN5boost4math8policies20raise_overflow_errorIeNS1_6policyINS1_13promote_floatILb0EEENS1_14promote_doubleILb0EEENS1_14default_policyES8_S8_S8_S8_S8_S8_S8_S8_S8_S8_EEEET_PKcSC_RKT0_(i8* getelementptr inbounds ([29 x i8], [29 x i8]* @.str.3.16422, i64 0, i64 0), i8* null, %"struct.std::__atomic_flag_base"* nonnull align 1 dereferenceable(1) %9)
  %78 = fneg contract x86_fp80 undef
  br label %121

CALL64pcrel32 @_ZN5boost4math8policies20raise_overflow_errorIeNS1_6policyINS1_13promote_floatILb0EEENS1_14promote_doubleILb0EEENS1_14default_policyES8_S8_S8_S8_S8_S8_S8_S8_S8_S8_EEEET_PKcSC_RKT0_, <regmask $bh $bl $bp $bph $bpl $bx $ebp $ebx $hbp $hbx $rbp $rbx $r12 $r13 $r14 $r15 $r12b $r13b $r14b $r15b $r12bh $r13bh $r14bh $r15bh $r12d $r13d $r14d $r15d $r12w $r13w $r14w $r15w $r12wh and 3 more...>, implicit $rsp, implicit $ssp, implicit $rdi, implicit $rsi, implicit $rdx, implicit-def $rsp, implicit-def $ssp, implicit-def $fp0
ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
%118:rfp80 = COPY $fp0
%13:rfp80 = nofpexcept LD_Fp080 implicit-def dead $fpsw, implicit $fpcw

In D104440#2829199, @LuoYuanke wrote:

BTW, do you think the patch to handle undef case in stackify pass reasonable?

In D104440#2829188, @LuoYuanke wrote:

Did you run llvm-extract to isolate the broken function first? bugpoint is not good at that.

Thanks Craig for the suggestion. After run "llvm-extract --recursive" and get the small file, I can't reproduce this issue. However I use -print-after-all to dump the IR of each pass. The undefine value is created in "processimpdefs" pass.

From

bb.11 (%ir-block.61):
; predecessors: %bb.10
  successors: %bb.26

  %164:gr64 = MOV64ri @.str.3.16422
  %165:gr32 = MOV32r0 implicit-def $eflags
  %166:gr64 = SUBREG_TO_REG 0, %165:gr32, %subreg.sub_32bit
  %167:gr64 = LEA64r %stack.6, 1, $noreg, 0, $noreg
  ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
  $rdi = COPY %164:gr64
  $rsi = COPY %166:gr64
  $rdx = COPY %167:gr64
  CALL64pcrel32 @_ZN5boost4math8policies20raise_overflow_errorIeNS1_6policyINS1_13promote_floatILb0EEENS1_14promote_doubleILb0EEENS1_14default_policyES8_S8_S8_S8_S8_S8_S8_S8_S8_S8_EEEET_PKcSC_RKT0_, <regmask $bh $bl $bp $bph $bpl $bx $ebp $ebx $hbp $hbx $rbp $rbx $r12 $r13 $r14 $r15 $r12b $r13b $r14b $r15b $r12bh $r13bh $r14bh $r15bh $r12d $r13d $r14d $r15d $r12w $r13w $r14w $r15w $r12wh and 3 more...>, implicit $rsp, implicit $ssp, implicit $rdi, implicit $rsi, implicit $rdx, implicit-def $fp0
  ADJCALLSTACKUP64 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
  %168:rfp80 = COPY $fp0
  %162:rfp80 = IMPLICIT_DEF
  %163:rfp80 = CHS_Fp80 %162:rfp80, implicit-def $fpsw
  JMP_1 %bb.26

To

bb.11 (%ir-block.61):
; predecessors: %bb.10
  successors: %bb.26

  %164:gr64 = MOV64ri @.str.3.16422
  %165:gr32 = MOV32r0 implicit-def $eflags
  %166:gr64 = SUBREG_TO_REG 0, %165:gr32, %subreg.sub_32bit
  %167:gr64 = LEA64r %stack.6, 1, $noreg, 0, $noreg
  ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
  $rdi = COPY %164:gr64
  $rsi = COPY %166:gr64
  $rdx = COPY %167:gr64
  CALL64pcrel32 @_ZN5boost4math8policies20raise_overflow_errorIeNS1_6policyINS1_13promote_floatILb0EEENS1_14promote_doubleILb0EEENS1_14default_policyES8_S8_S8_S8_S8_S8_S8_S8_S8_S8_EEEET_PKcSC_RKT0_, <regmask $bh $bl $bp $bph $bpl $bx $ebp $ebx $hbp $hbx $rbp $rbx $r12 $r13 $r14 $r15 $r12b $r13b $r14b $r15b $r12bh $r13bh $r14bh $r15bh $r12d $r13d $r14d $r15d $r12w $r13w $r14w $r15w $r12wh and 3 more...>, implicit $rsp, implicit $ssp, implicit $rdi, implicit $rsi, implicit $rdx, implicit-def $fp0
  ADJCALLSTACKUP64 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp
  %168:rfp80 = COPY $fp0
  %163:rfp80 = CHS_Fp80 undef %162:rfp80, implicit-def $fpsw
  JMP_1 %bb.26

Is this transform reasonable? "%162:rfp80 = IMPLICIT_DEF" is generated in ISel. I will look into why "%162:rfp80 = IMPLICIT_DEF" is generated.

In D104440#2829154, @LuoYuanke wrote:

In D104440#2828675, @craig.topper wrote:

Can you get IR and use bugpoint to reduce it? I'd really like to see the backend codegen that led to this case.

The .ll file is about 422 M. The reduce progress is slow. It has run 18 hours, but still not finished. Need more time to wait. Any parallel scheme to accelerate the reduce for bugpoint?

I did some more digging and it looks like ISD::UNDEF for X86 should be turned into ConstantFP<0> by LegalizeDAG. So I really need more information about how we got here.

I'd also like to know what happens if you add "(fneg undef) -> undef" fold to DAGCombiner::visitFNEG.

Can you get IR and use bugpoint to reduce it? I'd really like to see the backend codegen that led to this case.

LGTM

LGTM

Limit to cases where mul has a single use. There may be a better heuristic here,
but this is a simple starting point.

How did you get a CHS with an undef input?

There's a more generic optimization hiding here. Could we teach decomposeMulByConstant to emit (shl (sh1add X, X), C) to handle any constant of the form (3 << C). Similar for (shl (sh2add X, X)) to handle (5 << C), and (shl (sh3add X, X)) to handle (9 << C). If the multiply happens to be used by an add the existing patterns would combine the ADD and the SHL when possible.

LGTM

Add strictfp to caller in test case.

Address review feedback

Initialize member variable

In D104237#2819820, @frasercrmck wrote:

In D104237#2818945, @rogfer01 wrote:

In D104237#2817214, @craig.topper wrote:

Will frem need to be expanded to a loop in the expansion pass?

Would it be reasonable to expand it using the definition of fmod but vector-wise? It seems we'd need a division, a rounding to integer towards zero, convert back to float, a mutiplication and then a subtraction.

Will frem need to be expanded to a loop in the expansion pass?

LGTM

In D104178#2815069, @RKSimon wrote:

LGTM - does this need to be back ported to 12.x?

Fixed typo in comment

LGTM

LGTM

I don't what Intel's original failure looked like, but here's a test that should reproduce this with -run-pass=machinelicm https://reviews.llvm.org/P8267 needs more cleanup.

It would be nice to have a test, but this change seems ok.

In D104061#2811804, @rnk wrote:

That's a good point, if this attribute disables vectorization of integer math, the docs should say as much.