[llvm] r295004 - [X86] Add MXCSR register
Kaylor, Andrew via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 24 12:32:21 PST 2017
My commit missed a few lines where I added MXCSR to a register class and says it can't be copied or allocated. Would that cause this type of failure?
-----Original Message-----
From: davide.italiano at gmail.com [mailto:davide.italiano at gmail.com] On Behalf Of Davide Italiano
Sent: Friday, February 24, 2017 12:28 PM
To: Reid Kleckner <rnk at google.com>
Cc: Kaylor, Andrew <andrew.kaylor at intel.com>; llvm-commits <llvm-commits at lists.llvm.org>
Subject: Re: [llvm] r295004 - [X86] Add MXCSR register
This unforunately broke at least one of our internal codebases. I'm getting a repro for you.
It fails in the register allocator with a bunch of:
[...]
*** Bad machine code: Using an undefined physical register ***
- function: tinkywinky
- basic block: BB#260 (0x66474880)
- instruction: VSTMXCSR
- operand 5: %MXCSR<imp-use,kill>
On Wed, Feb 15, 2017 at 11:07 AM, Reid Kleckner via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> Splitting up MXCSR makes sense. We should do the same thing for EFLAGS
> at some point.
>
> On Wed, Feb 15, 2017 at 10:52 AM, Kaylor, Andrew
> <andrew.kaylor at intel.com>
> wrote:
>>
>> When I looked at this I noticed that there are some interesting
>> things going on, like “fpsw” being suppressed from the clobber list.
>> This caught my eye because the GCCRegNames array doesn’t have “fpsw”, calling it “fpsr”
>> instead. I was looking because that’s the x87 partial-equivalent of MXCSR.
>> So if clang hadn’t filtered it, a name translation would have been
>> necessary. If this were some other register for which clang needed
>> clobber information and there were a naming mismatch between LLVM and
>> GCC the current assertion would catch that. I doubt that will come
>> up very often, but it seems like a useful behavior.
>>
>>
>>
>> The other reason I’m interested in this now is that I’m exploring the
>> idea of representing MXCSR in LLVM as if it were two separate
>> registers. Since it can’t be used as an operand to any instruction I
>> should be able to create whatever useful fiction I want, and
>> separating the control bits from the status bits will be useful for
>> restricting code motion when it must be restricted (like not moving
>> FP operations past instructions that change the rounding mode) while
>> allowing code motion that would be blocked by treating control and
>> status bits as a single register but doesn’t actually need to be blocked (like moving FP operations past other FP operations).
>>
>>
>>
>> If we did as you suggest and just replaced the assertion with a
>> filter that would handle the fictional registers I have in mind by
>> simply ignoring them. I haven’t followed the trail of what happens
>> to the clobber information clang is trying to create there. Would it
>> allow, for instance, an inline assembly block that contains a LDMXCSR
>> instruction to be hoisted above an inline assembly block that contains a STMXCSR instruction?
>>
>>
>>
>> -Andy
>>
>>
>>
>> From: Reid Kleckner [mailto:rnk at google.com]
>> Sent: Wednesday, February 15, 2017 10:28 AM
>>
>>
>> To: Kaylor, Andrew <andrew.kaylor at intel.com>
>> Cc: llvm-commits <llvm-commits at lists.llvm.org>
>> Subject: Re: [llvm] r295004 - [X86] Add MXCSR register
>>
>>
>>
>> I fixed this on the clang side in r295107, but forgot to update this
>> thread. I think the clang assertion is probably buggy and we should
>> essentially pass inferred clobbers through a whitelist of known
>> inline asm clobbers.
>>
>>
>>
>> On Tue, Feb 14, 2017 at 3:01 PM, Kaylor, Andrew
>> <andrew.kaylor at intel.com>
>> wrote:
>>
>> Sorry about that. I’ll look into it and see if there’s something I
>> should do about it in the .td files. Thanks for the small reduced test case!
>>
>>
>>
>> -Andy
>>
>>
>>
>> From: Reid Kleckner [mailto:rnk at google.com]
>> Sent: Tuesday, February 14, 2017 1:48 PM
>> To: Kaylor, Andrew <andrew.kaylor at intel.com>
>> Cc: llvm-commits <llvm-commits at lists.llvm.org>
>> Subject: Re: [llvm] r295004 - [X86] Add MXCSR register
>>
>>
>>
>> This broke building Chrome on Windows because it affects Intel inline
>> asm clobber inference. Reduction:
>>
>>
>>
>> void f() {
>>
>> int u;
>>
>> __asm {
>>
>> fxrstor u
>>
>> };
>>
>> }
>>
>>
>>
>> Compiling this gives:
>>
>> Assertion failed: isValidGCCRegisterName(Name) && "Invalid register
>> passed in", file ..\tools\clang\lib\Basic\TargetInfo.cpp, line 415
>>
>>
>>
>> I'm drafting a fix for clang now.
>>
>>
>>
>> On Mon, Feb 13, 2017 at 3:38 PM, Andrew Kaylor via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: akaylor
>> Date: Mon Feb 13 17:38:52 2017
>> New Revision: 295004
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=295004&view=rev
>> Log:
>> [X86] Add MXCSR register
>>
>> This adds MXCSR to the set of recognized registers for X86 targets
>> and updates the instructions that read or write it. I do not intend
>> for all of the various floating point instructions that implicitly
>> use the control bits or update the status bits of this register to
>> ever have that usage modeled by default. However, when constrained
>> floating point modes (such as strict FP exception status modeling or
>> dynamic rounding modes) are enabled, implicit use/def information for MXCSR will be added to those instructions.
>>
>> Until those additional updates are made this should cause (almost?)
>> no functional changes. Theoretically, this will prevent instructions
>> like LDMXCSR and STMXCSR from being moved past one another, but that
>> should be prevented anyway and I haven't found a case where it is happening now.
>>
>> Differential Revision: https://reviews.llvm.org/D29903
>>
>>
>> Modified:
>> llvm/trunk/lib/Target/X86/X86InstrFPStack.td
>> llvm/trunk/lib/Target/X86/X86InstrSSE.td
>> llvm/trunk/lib/Target/X86/X86RegisterInfo.td
>> llvm/trunk/test/CodeGen/X86/ipra-reg-usage.ll
>>
>> Modified: llvm/trunk/lib/Target/X86/X86InstrFPStack.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86Inst
>> rFPStack.td?rev=295004&r1=295003&r2=295004&view=diff
>>
>> =====================================================================
>> =========
>> --- llvm/trunk/lib/Target/X86/X86InstrFPStack.td (original)
>> +++ llvm/trunk/lib/Target/X86/X86InstrFPStack.td Mon Feb 13 17:38:52
>> +++ 2017
>> @@ -667,16 +667,21 @@ def FSCALE : I<0xD9, MRM_FD, (outs), (in def
>> FCOMPP : I<0xDE, MRM_D9, (outs), (ins), "fcompp", [], IIC_FCOMPP>;
>>
>> let Predicates = [HasFXSR] in {
>> - def FXSAVE : I<0xAE, MRM0m, (outs), (ins opaque512mem:$dst),
>> + let Uses = [MXCSR] in {
>> + def FXSAVE : I<0xAE, MRM0m, (outs), (ins opaque512mem:$dst),
>> "fxsave\t$dst", [(int_x86_fxsave addr:$dst)],
>> IIC_FXSAVE>, TB;
>> - def FXSAVE64 : RI<0xAE, MRM0m, (outs), (ins opaque512mem:$dst),
>> - "fxsave64\t$dst", [(int_x86_fxsave64 addr:$dst)],
>> - IIC_FXSAVE>, TB, Requires<[In64BitMode]>;
>> - def FXRSTOR : I<0xAE, MRM1m, (outs), (ins opaque512mem:$src),
>> - "fxrstor\t$src", [(int_x86_fxrstor addr:$src)],
>> IIC_FXRSTOR>, TB;
>> - def FXRSTOR64 : RI<0xAE, MRM1m, (outs), (ins opaque512mem:$src),
>> - "fxrstor64\t$src", [(int_x86_fxrstor64 addr:$src)],
>> - IIC_FXRSTOR>, TB, Requires<[In64BitMode]>;
>> + def FXSAVE64 : RI<0xAE, MRM0m, (outs), (ins opaque512mem:$dst),
>> + "fxsave64\t$dst", [(int_x86_fxsave64 addr:$dst)],
>> + IIC_FXSAVE>, TB, Requires<[In64BitMode]>; } let
>> + Defs = [MXCSR] in {
>> + def FXRSTOR : I<0xAE, MRM1m, (outs), (ins opaque512mem:$src),
>> + "fxrstor\t$src", [(int_x86_fxrstor addr:$src)],
>> IIC_FXRSTOR>,
>> + TB;
>> + def FXRSTOR64 : RI<0xAE, MRM1m, (outs), (ins opaque512mem:$src),
>> + "fxrstor64\t$src", [(int_x86_fxrstor64 addr:$src)],
>> + IIC_FXRSTOR>, TB, Requires<[In64BitMode]>; }
>> } // Predicates = [FeatureFXSR]
>> } // SchedRW
>>
>>
>> Modified: llvm/trunk/lib/Target/X86/X86InstrSSE.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86Inst
>> rSSE.td?rev=295004&r1=295003&r2=295004&view=diff
>>
>> =====================================================================
>> =========
>> --- llvm/trunk/lib/Target/X86/X86InstrSSE.td (original)
>> +++ llvm/trunk/lib/Target/X86/X86InstrSSE.td Mon Feb 13 17:38:52 2017
>> @@ -3743,20 +3743,24 @@ def : Pat<(X86MFence), (MFENCE)>; // SSE 1 &
>> 2 - Load/Store XCSR register
>>
>> //===----------------------------------------------------------------
>> ------===//
>>
>> -def VLDMXCSR : VPSI<0xAE, MRM2m, (outs), (ins i32mem:$src),
>> - "ldmxcsr\t$src", [(int_x86_sse_ldmxcsr addr:$src)],
>> - IIC_SSE_LDMXCSR>, VEX, Sched<[WriteLoad]>;
>> -def VSTMXCSR : VPSI<0xAE, MRM3m, (outs), (ins i32mem:$dst),
>> - "stmxcsr\t$dst", [(int_x86_sse_stmxcsr addr:$dst)],
>> - IIC_SSE_STMXCSR>, VEX, Sched<[WriteStore]>;
>> +let Defs = [MXCSR] in
>> + def VLDMXCSR : VPSI<0xAE, MRM2m, (outs), (ins i32mem:$src),
>> + "ldmxcsr\t$src", [(int_x86_sse_ldmxcsr addr:$src)],
>> + IIC_SSE_LDMXCSR>, VEX, Sched<[WriteLoad]>; let Uses
>> += [MXCSR] in
>> + def VSTMXCSR : VPSI<0xAE, MRM3m, (outs), (ins i32mem:$dst),
>> + "stmxcsr\t$dst", [(int_x86_sse_stmxcsr addr:$dst)],
>> + IIC_SSE_STMXCSR>, VEX, Sched<[WriteStore]>;
>>
>> let Predicates = [UseSSE1] in {
>> -def LDMXCSR : I<0xAE, MRM2m, (outs), (ins i32mem:$src),
>> - "ldmxcsr\t$src", [(int_x86_sse_ldmxcsr addr:$src)],
>> - IIC_SSE_LDMXCSR>, TB, Sched<[WriteLoad]>;
>> -def STMXCSR : I<0xAE, MRM3m, (outs), (ins i32mem:$dst),
>> - "stmxcsr\t$dst", [(int_x86_sse_stmxcsr addr:$dst)],
>> - IIC_SSE_STMXCSR>, TB, Sched<[WriteStore]>;
>> + let Defs = [MXCSR] in
>> + def LDMXCSR : I<0xAE, MRM2m, (outs), (ins i32mem:$src),
>> + "ldmxcsr\t$src", [(int_x86_sse_ldmxcsr addr:$src)],
>> + IIC_SSE_LDMXCSR>, TB, Sched<[WriteLoad]>; let
>> + Uses = [MXCSR] in
>> + def STMXCSR : I<0xAE, MRM3m, (outs), (ins i32mem:$dst),
>> + "stmxcsr\t$dst", [(int_x86_sse_stmxcsr addr:$dst)],
>> + IIC_SSE_STMXCSR>, TB, Sched<[WriteStore]>;
>> }
>>
>>
>> //===----------------------------------------------------------------
>> -----===//
>>
>> Modified: llvm/trunk/lib/Target/X86/X86RegisterInfo.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86Regi
>> sterInfo.td?rev=295004&r1=295003&r2=295004&view=diff
>>
>> =====================================================================
>> =========
>> --- llvm/trunk/lib/Target/X86/X86RegisterInfo.td (original)
>> +++ llvm/trunk/lib/Target/X86/X86RegisterInfo.td Mon Feb 13 17:38:52
>> +++ 2017
>> @@ -254,6 +254,9 @@ def FPSW : X86Reg<"fpsw", 0>; // Status flags
>> register def EFLAGS : X86Reg<"flags", 0>;
>>
>> +// SSE floating point control/status register def MXCSR :
>> +X86Reg<"mxcsr", 0>;
>> +
>> // Segment registers
>> def CS : X86Reg<"cs", 1>;
>> def DS : X86Reg<"ds", 3>;
>>
>> Modified: llvm/trunk/test/CodeGen/X86/ipra-reg-usage.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/ipra-
>> reg-usage.ll?rev=295004&r1=295003&r2=295004&view=diff
>>
>> =====================================================================
>> =========
>> --- llvm/trunk/test/CodeGen/X86/ipra-reg-usage.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/ipra-reg-usage.ll Mon Feb 13 17:38:52
>> +++ 2017
>> @@ -3,7 +3,7 @@
>> target triple = "x86_64-unknown-unknown"
>> declare void @bar1()
>> define preserve_allcc void @foo()#0 { -; CHECK: foo Clobbered
>> Registers: CS DS EFLAGS EIP EIZ ES FPSW FS GS IP RIP RIZ SS BND0 BND1
>> BND2 BND3 CR0 CR1 CR2 CR3 CR4 CR5 CR6 CR7 CR8 CR9 CR10
>> CR11 CR12 CR13 CR14 CR15 DR0 DR1 DR2 DR3 DR4 DR5 DR6 DR7 DR8 DR9 DR10
>> DR11
>> DR12 DR13 DR14 DR15 FP0 FP1 FP2 FP3 FP4 FP5 FP6 FP7 K0 K1 K2 K3 K4 K5
>> K6 K7
>> MM0 MM1 MM2 MM3 MM4 MM5 MM6 MM7 R11 ST0 ST1 ST2 ST3 ST4 ST5 ST6 ST7
>> XMM16
>> XMM17 XMM18 XMM19 XMM20 XMM21 XMM22 XMM23 XMM24 XMM25 XMM26 XMM27
>> XMM28
>> XMM29 XMM30 XMM31 YMM0 YMM1 YMM2 YMM3 YMM4 YMM5 YMM6 YMM7 YMM8 YMM9
>> YMM10
>> YMM11 YMM12 YMM13 YMM14 YMM15 YMM16 YMM17 YMM18 YMM19 YMM20 YMM21
>> YMM22
>> YMM23 YMM24 YMM25 YMM26 YMM27 YMM28 YMM29 YMM30 YMM31 ZMM0 ZMM1 ZMM2
>> ZMM3
>> ZMM4 ZMM5 ZMM6 ZMM7 ZMM8 ZMM9 ZMM10 ZMM11 ZMM12 ZMM13 ZMM14 ZMM15
>> ZMM16
>> ZMM17 ZMM18 ZMM19 ZMM20 ZMM21 ZMM22 ZMM23 ZMM24 ZMM25 ZMM26 ZMM27
>> ZMM28
>> ZMM29 ZMM30 ZMM31 R11B R11D R11W
>> +; CHECK: foo Clobbered Registers: CS DS EFLAGS EIP EIZ ES FPSW FS GS
>> +IP
>> MXCSR RIP RIZ SS BND0 BND1 BND2 BND3 CR0 CR1 CR2 CR3 CR4 CR5 CR6 CR7
>> CR8 CR9
>> CR10 CR11 CR12 CR13 CR14 CR15 DR0 DR1 DR2 DR3 DR4 DR5 DR6 DR7 DR8 DR9
>> DR10
>> DR11 DR12 DR13 DR14 DR15 FP0 FP1 FP2 FP3 FP4 FP5 FP6 FP7 K0 K1 K2 K3
>> K4 K5
>> K6 K7 MM0 MM1 MM2 MM3 MM4 MM5 MM6 MM7 R11 ST0 ST1 ST2 ST3 ST4 ST5 ST6
>> ST7
>> XMM16 XMM17 XMM18 XMM19 XMM20 XMM21 XMM22 XMM23 XMM24 XMM25 XMM26
>> XMM27
>> XMM28 XMM29 XMM30 XMM31 YMM0 YMM1 YMM2 YMM3 YMM4 YMM5 YMM6 YMM7 YMM8
>> YMM9
>> YMM10 YMM11 YMM12 YMM13 YMM14 YMM15 YMM16 YMM17 YMM18 YMM19 YMM20
>> YMM21
>> YMM22 YMM23 YMM24 YMM25 YMM26 YMM27 YMM28 YMM29 YMM30 YMM31 ZMM0 ZMM1
>> ZMM2
>> ZMM3 ZMM4 ZMM5 ZMM6 ZMM7 ZMM8 ZMM9 ZMM10 ZMM11 ZMM12 ZMM13 ZMM14
>> ZMM15 ZMM16
>> ZMM17 ZMM18 ZMM19 ZMM20 ZMM21 ZMM22 ZMM23 ZMM24 ZMM25 ZMM26 ZMM27
>> ZMM28
>> ZMM29 ZMM30 ZMM31 R11B R11D R11W
>> call void @bar1()
>> call void @bar2()
>> ret void
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>>
>>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
--
Davide
"There are no solved problems; there are only problems that are more or less solved" -- Henri Poincare
More information about the llvm-commits
mailing list