[llvm] r295004 - [X86] Add MXCSR register

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 24 12:28:21 PST 2017


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/X86InstrFPStack.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/X86InstrSSE.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/X86RegisterInfo.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