You can take my patch

Edit commit message.

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.

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;
 }

I think because I use the

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

Are you running the large and small cases the same way? Have looked at the SelectionDAG debug logs for the affected basic block in the large case?

I was able to trigger the error with llc -O2 -fast-isel on a simple test. So that is a path to create this but you haven’t answered if fast isel is being used in your case.

In D104440#2829202, @craig.topper wrote:

In D104440#2829199, @LuoYuanke wrote:

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

No I do not. I have no reason to believe fneg or one arg instructions are the only things that could be effected. We need to understand why this is happening because isel was trying to prevent this.

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

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?

Here is IR.

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

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.

Address Craig's comments.

In D104440#2825925, @craig.topper wrote:

How did you get a CHS with an undef input?

Update test case to pass expensive check.

In D104037#2813726, @craig.topper wrote:

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.

I hacked the MIR just before machinelicm by sinking the CMP64mi32 and SETCCr into the loop. That makes MachineLICM want to unfold it since the load part is invariant being from a constant global.

Apply Craig's test case. Many thanks to Craig. :)

Thank @lebedev.ri. This is triggered by our internal code, the same test case passes with llvm trunk code. BTW, do you think we need to check immediate before getting it?

LGTM. Thank you!

We may add description on the intrinsic in docs/LangRef.rst.

Address Pengfei's comments.

Remove tab.

LGTM.

Fix some descriptions.

LGTM.

Is there any test case for it?

Address Craig's comments.

Address Roman's comments.

Remove attribute in test case.

Address Pengfei's comments. Add amx description to BitCodeFormat.html.

LGTM. Thank you!

Address Florian and Pengfei's comments.

LGTM. But wait one or two days to see if there is more comments from Craig and HJ.

LGTM. Thanks!

Reformat it.

LGMT. Thank you!

Perhaps we need more comments and more test cases (maybe in a sperate file) to cover those scenario.

In D99708#2663989, @craig.topper wrote:

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Unfortunately this is not possible to use an opaque type with the AMX intrinsics at the moment, because of the way they are define. It is possible to use opaque types with intrinsics in general though, e.g. see https://llvm.godbolt.org/z/Ezhf6535c

My point is, you should be able to adjust the definitions of the AMX intrinsics and then just replace all occurrences of x86_amx in your examples with a opaque type you define in the module. But as I said initially, you don't need to do everything at once (and you probably shouldn't). I'd start with addressing the bitcast issue and tackle the x86_amx type itself once that is done.

(And I am also not saying that it definitely needs to be removed, only that if it should be kept in the long run, it would be good to specify it in the LangRef and should have a good justification, especially if there are no instructions that do anything meaningful with values of the type other than take it as arguments and return values. Opaque types are a suggestion for an alternative that *may* be viable without a dedicated first-class type)

Whether to further optimizations are correct is a different problem, but we need a specification for the builtins, intrinsics and the type before going any further in that direction.

I think you need to set the input to LLVM IR: https://gcc.godbolt.org/z/WexMjsas9

You should be able to use opaque types with overloaded intrinsics. I don't think you define an intrinsic to take a specific opaque type (because it's not known up front).

I think that point was not really clear during the discussion. Using load <256 x i32> to lower __tile_loadd() would indeed be incorrect. But I don't think that's happening at the moment, at least going from a simple example https://gcc.godbolt.org/z/KT5rczn8j

The load/store <256 x i32> is generated by front-end, because in C language tile is a vector <256 x i32>. The load/store <256 x i32> is transformed to llvm.x86.tileloadd64.internal/llvm.x86.tilestored64.internal in lib/Target/X86/X86LowerAMXType.cpp if the load result is to be an operand of amx intrinsics or the store value is returned from amx intrinsics.