[llvm] r295004 - [X86] Add MXCSR register

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 24 14:10:45 PST 2017


On Fri, Feb 24, 2017 at 12:32 PM, Kaylor, Andrew
<andrew.kaylor at intel.com> wrote:
> 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?
>

Could be. Do you have a patch I can try?

--
Davide

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



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