| Message ID | CAFULd4ZpeFBnjScbwxdDQb9pQFwbgcQvt6FVKTX2RCnbrzcKNA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
2013/11/20 Uros Bizjak <ubizjak@gmail.com>: > On Wed, Nov 20, 2013 at 1:32 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: > >>> >> > Here is a patch to add size relocation and instruction to obtain object's size in i386 target. >>> >> >>> >> +(define_insn "move_size_reloc_<mode>" >>> >> + [(set (match_operand:SWI48 0 "register_operand" "=r") >>> >> + (match_operand:<MODE> 1 "size_relocation" "Z"))] >>> >> + "" >>> >> +{ >>> >> + return "mov{<imodesuffix>}\t{%1, %0|%0, %1}"; >>> >> >>> >> Please don't change x86_64_immediate_operand just to use "Z" >>> >> constraint The predicate is used in a couple of other places that for >>> >> sure don't accept your change. >>> >> >>> >> Better write this insn in an explicit way (see for example >>> >> tls_initial_exec_64_sun). Something like: >>> >> >>> >> (define_insn "move_size_reloc_<mode>" >>> >> [(set (match_operand:SWI48 0 "register_operand" "=r") >>> >> (unspec:SWI48 >>> >> [(match_operand 1 "symbolic_operand" "..." )] >>> >> UNSPEC_SIZEOF))] >>> >> "" >>> >> "mov{<imodesuffix>}\t{%a1@SIZE, %0|%0, %a1@SIZE}") >>> >> >>> >> You will probably need to define new operand 1 predicate and constraint. >>> >> >>> >> Uros. >>> > >>> > Hi, Uros! Thanks for comments! Here is what I got trying to follow your suggestion. Does it look better? >>> >>> You actually don't need any operand modifiers in the insn template. Simply use: >>> >>> "mov{<imodesuffix>}\t{%1@SIZE, %0|%0, %1@SIZE}" >>> >>> and you will automatically get >>> >>> "movl $zzz, %eax" or "mov %eax, OFFSET FLAT: zzz". >>> >>> Since your pattern allows only symbolic_operand, there is no reload, >>> so you can avoid the constraint alltogether. >>> >>> BTW: Did you consider various -mcmodel=... options? For DImode moves, >>> you should check x86_64_zext_immediate_operand predicate and output >>> either "movl $zzz, %eax" or "movabs $zzz, %rax". There is no movq with >>> 64bit immediate. Please see movdi pattern. >> >> Yep, for large objects it may work wrongly. Does anyone use static objects >4Gb? :) >> >> Large address does not mean large object but seems we have to be conservative here. I added x86_64_zext_immediate_operand check with additional CM_KERNEL check because in this model object size should always fit 32 bits. > > IMO, we should list code models that support object sizes > 31bits for > 64bit target. The object size in small models will never be > 31bits > (and never negative), so we can use movl unconditionally. For medium models x86_64_zext_immediate_operand returns true for object is known to go to lower 2Gb space. It should allow us to use movl. Why do you always emit movabs for medium model? Ilya > > Please try attached patch. > > Uros.
On Wed, Nov 20, 2013 at 5:11 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: > 2013/11/20 Uros Bizjak <ubizjak@gmail.com>: >> On Wed, Nov 20, 2013 at 1:32 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: >> >>>> >> > Here is a patch to add size relocation and instruction to obtain object's size in i386 target. >>>> >> >>>> >> +(define_insn "move_size_reloc_<mode>" >>>> >> + [(set (match_operand:SWI48 0 "register_operand" "=r") >>>> >> + (match_operand:<MODE> 1 "size_relocation" "Z"))] >>>> >> + "" >>>> >> +{ >>>> >> + return "mov{<imodesuffix>}\t{%1, %0|%0, %1}"; >>>> >> >>>> >> Please don't change x86_64_immediate_operand just to use "Z" >>>> >> constraint The predicate is used in a couple of other places that for >>>> >> sure don't accept your change. >>>> >> >>>> >> Better write this insn in an explicit way (see for example >>>> >> tls_initial_exec_64_sun). Something like: >>>> >> >>>> >> (define_insn "move_size_reloc_<mode>" >>>> >> [(set (match_operand:SWI48 0 "register_operand" "=r") >>>> >> (unspec:SWI48 >>>> >> [(match_operand 1 "symbolic_operand" "..." )] >>>> >> UNSPEC_SIZEOF))] >>>> >> "" >>>> >> "mov{<imodesuffix>}\t{%a1@SIZE, %0|%0, %a1@SIZE}") >>>> >> >>>> >> You will probably need to define new operand 1 predicate and constraint. >>>> >> >>>> >> Uros. >>>> > >>>> > Hi, Uros! Thanks for comments! Here is what I got trying to follow your suggestion. Does it look better? >>>> >>>> You actually don't need any operand modifiers in the insn template. Simply use: >>>> >>>> "mov{<imodesuffix>}\t{%1@SIZE, %0|%0, %1@SIZE}" >>>> >>>> and you will automatically get >>>> >>>> "movl $zzz, %eax" or "mov %eax, OFFSET FLAT: zzz". >>>> >>>> Since your pattern allows only symbolic_operand, there is no reload, >>>> so you can avoid the constraint alltogether. >>>> >>>> BTW: Did you consider various -mcmodel=... options? For DImode moves, >>>> you should check x86_64_zext_immediate_operand predicate and output >>>> either "movl $zzz, %eax" or "movabs $zzz, %rax". There is no movq with >>>> 64bit immediate. Please see movdi pattern. >>> >>> Yep, for large objects it may work wrongly. Does anyone use static objects >4Gb? :) >>> >>> Large address does not mean large object but seems we have to be conservative here. I added x86_64_zext_immediate_operand check with additional CM_KERNEL check because in this model object size should always fit 32 bits. >> >> IMO, we should list code models that support object sizes > 31bits for >> 64bit target. The object size in small models will never be > 31bits >> (and never negative), so we can use movl unconditionally. > > For medium models x86_64_zext_immediate_operand returns true for > object is known to go to lower 2Gb space. It should allow us to use > movl. Why do you always emit movabs for medium model? CM_MEDIUM has unlimited data size. i386-opts.h: CM_MEDIUM, /* Assumes code fits in the low 31 bits; data unlimited. */ The x86_64_zext_immediate_operand allows _address_ to be loaded by movl. The @SIZE reloc returns object size, which is unlimited and has no connection to its address. For CM_MEDIUM, x86_64_zext_immediate_operand allows: return (ix86_cmodel == CM_SMALL || (ix86_cmodel == CM_MEDIUM && !SYMBOL_REF_FAR_ADDR_P (op))); Uros.
2013/11/20 Uros Bizjak <ubizjak@gmail.com>: > On Wed, Nov 20, 2013 at 5:11 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: >> 2013/11/20 Uros Bizjak <ubizjak@gmail.com>: >>> On Wed, Nov 20, 2013 at 1:32 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: >>> >>>>> >> > Here is a patch to add size relocation and instruction to obtain object's size in i386 target. >>>>> >> >>>>> >> +(define_insn "move_size_reloc_<mode>" >>>>> >> + [(set (match_operand:SWI48 0 "register_operand" "=r") >>>>> >> + (match_operand:<MODE> 1 "size_relocation" "Z"))] >>>>> >> + "" >>>>> >> +{ >>>>> >> + return "mov{<imodesuffix>}\t{%1, %0|%0, %1}"; >>>>> >> >>>>> >> Please don't change x86_64_immediate_operand just to use "Z" >>>>> >> constraint The predicate is used in a couple of other places that for >>>>> >> sure don't accept your change. >>>>> >> >>>>> >> Better write this insn in an explicit way (see for example >>>>> >> tls_initial_exec_64_sun). Something like: >>>>> >> >>>>> >> (define_insn "move_size_reloc_<mode>" >>>>> >> [(set (match_operand:SWI48 0 "register_operand" "=r") >>>>> >> (unspec:SWI48 >>>>> >> [(match_operand 1 "symbolic_operand" "..." )] >>>>> >> UNSPEC_SIZEOF))] >>>>> >> "" >>>>> >> "mov{<imodesuffix>}\t{%a1@SIZE, %0|%0, %a1@SIZE}") >>>>> >> >>>>> >> You will probably need to define new operand 1 predicate and constraint. >>>>> >> >>>>> >> Uros. >>>>> > >>>>> > Hi, Uros! Thanks for comments! Here is what I got trying to follow your suggestion. Does it look better? >>>>> >>>>> You actually don't need any operand modifiers in the insn template. Simply use: >>>>> >>>>> "mov{<imodesuffix>}\t{%1@SIZE, %0|%0, %1@SIZE}" >>>>> >>>>> and you will automatically get >>>>> >>>>> "movl $zzz, %eax" or "mov %eax, OFFSET FLAT: zzz". >>>>> >>>>> Since your pattern allows only symbolic_operand, there is no reload, >>>>> so you can avoid the constraint alltogether. >>>>> >>>>> BTW: Did you consider various -mcmodel=... options? For DImode moves, >>>>> you should check x86_64_zext_immediate_operand predicate and output >>>>> either "movl $zzz, %eax" or "movabs $zzz, %rax". There is no movq with >>>>> 64bit immediate. Please see movdi pattern. >>>> >>>> Yep, for large objects it may work wrongly. Does anyone use static objects >4Gb? :) >>>> >>>> Large address does not mean large object but seems we have to be conservative here. I added x86_64_zext_immediate_operand check with additional CM_KERNEL check because in this model object size should always fit 32 bits. >>> >>> IMO, we should list code models that support object sizes > 31bits for >>> 64bit target. The object size in small models will never be > 31bits >>> (and never negative), so we can use movl unconditionally. >> >> For medium models x86_64_zext_immediate_operand returns true for >> object is known to go to lower 2Gb space. It should allow us to use >> movl. Why do you always emit movabs for medium model? > > CM_MEDIUM has unlimited data size. > > i386-opts.h: CM_MEDIUM, /* Assumes code fits in the low 31 > bits; data unlimited. */ > > The x86_64_zext_immediate_operand allows _address_ to be loaded by > movl. The @SIZE reloc returns object size, which is unlimited and has > no connection to its address. For CM_MEDIUM, > x86_64_zext_immediate_operand allows: > > return (ix86_cmodel == CM_SMALL > || (ix86_cmodel == CM_MEDIUM > && !SYMBOL_REF_FAR_ADDR_P (op))); Yes, but large symbols never have SYMBOL_FLAG_FAR_ADDR set. Ilya > > Uros.
2013/11/20 Ilya Enkovich <enkovich.gnu@gmail.com>: > 2013/11/20 Uros Bizjak <ubizjak@gmail.com>: >> On Wed, Nov 20, 2013 at 5:11 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: >>> 2013/11/20 Uros Bizjak <ubizjak@gmail.com>: >>>> On Wed, Nov 20, 2013 at 1:32 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: >>>> >>>>>> >> > Here is a patch to add size relocation and instruction to obtain object's size in i386 target. >>>>>> >> >>>>>> >> +(define_insn "move_size_reloc_<mode>" >>>>>> >> + [(set (match_operand:SWI48 0 "register_operand" "=r") >>>>>> >> + (match_operand:<MODE> 1 "size_relocation" "Z"))] >>>>>> >> + "" >>>>>> >> +{ >>>>>> >> + return "mov{<imodesuffix>}\t{%1, %0|%0, %1}"; >>>>>> >> >>>>>> >> Please don't change x86_64_immediate_operand just to use "Z" >>>>>> >> constraint The predicate is used in a couple of other places that for >>>>>> >> sure don't accept your change. >>>>>> >> >>>>>> >> Better write this insn in an explicit way (see for example >>>>>> >> tls_initial_exec_64_sun). Something like: >>>>>> >> >>>>>> >> (define_insn "move_size_reloc_<mode>" >>>>>> >> [(set (match_operand:SWI48 0 "register_operand" "=r") >>>>>> >> (unspec:SWI48 >>>>>> >> [(match_operand 1 "symbolic_operand" "..." )] >>>>>> >> UNSPEC_SIZEOF))] >>>>>> >> "" >>>>>> >> "mov{<imodesuffix>}\t{%a1@SIZE, %0|%0, %a1@SIZE}") >>>>>> >> >>>>>> >> You will probably need to define new operand 1 predicate and constraint. >>>>>> >> >>>>>> >> Uros. >>>>>> > >>>>>> > Hi, Uros! Thanks for comments! Here is what I got trying to follow your suggestion. Does it look better? >>>>>> >>>>>> You actually don't need any operand modifiers in the insn template. Simply use: >>>>>> >>>>>> "mov{<imodesuffix>}\t{%1@SIZE, %0|%0, %1@SIZE}" >>>>>> >>>>>> and you will automatically get >>>>>> >>>>>> "movl $zzz, %eax" or "mov %eax, OFFSET FLAT: zzz". >>>>>> >>>>>> Since your pattern allows only symbolic_operand, there is no reload, >>>>>> so you can avoid the constraint alltogether. >>>>>> >>>>>> BTW: Did you consider various -mcmodel=... options? For DImode moves, >>>>>> you should check x86_64_zext_immediate_operand predicate and output >>>>>> either "movl $zzz, %eax" or "movabs $zzz, %rax". There is no movq with >>>>>> 64bit immediate. Please see movdi pattern. >>>>> >>>>> Yep, for large objects it may work wrongly. Does anyone use static objects >4Gb? :) >>>>> >>>>> Large address does not mean large object but seems we have to be conservative here. I added x86_64_zext_immediate_operand check with additional CM_KERNEL check because in this model object size should always fit 32 bits. >>>> >>>> IMO, we should list code models that support object sizes > 31bits for >>>> 64bit target. The object size in small models will never be > 31bits >>>> (and never negative), so we can use movl unconditionally. >>> >>> For medium models x86_64_zext_immediate_operand returns true for >>> object is known to go to lower 2Gb space. It should allow us to use >>> movl. Why do you always emit movabs for medium model? >> >> CM_MEDIUM has unlimited data size. >> >> i386-opts.h: CM_MEDIUM, /* Assumes code fits in the low 31 >> bits; data unlimited. */ >> >> The x86_64_zext_immediate_operand allows _address_ to be loaded by >> movl. The @SIZE reloc returns object size, which is unlimited and has >> no connection to its address. For CM_MEDIUM, >> x86_64_zext_immediate_operand allows: >> >> return (ix86_cmodel == CM_SMALL >> || (ix86_cmodel == CM_MEDIUM >> && !SYMBOL_REF_FAR_ADDR_P (op))); > > Yes, but large symbols never have SYMBOL_FLAG_FAR_ADDR set. Sorry, I mean all large symbols have this flag set. > > Ilya > >> >> Uros.
On Wed, Nov 20, 2013 at 5:33 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: >>> CM_MEDIUM has unlimited data size. >>> >>> i386-opts.h: CM_MEDIUM, /* Assumes code fits in the low 31 >>> bits; data unlimited. */ >>> >>> The x86_64_zext_immediate_operand allows _address_ to be loaded by >>> movl. The @SIZE reloc returns object size, which is unlimited and has >>> no connection to its address. For CM_MEDIUM, >>> x86_64_zext_immediate_operand allows: >>> >>> return (ix86_cmodel == CM_SMALL >>> || (ix86_cmodel == CM_MEDIUM >>> && !SYMBOL_REF_FAR_ADDR_P (op))); >> >> Yes, but large symbols never have SYMBOL_FLAG_FAR_ADDR set. > > Sorry, I mean all large symbols have this flag set. This flag marks the fact that object size is bigger than -mlarge-data-threshold and goes into large section. It is true that UInt option argument currently overflows at 4G, and consequently all sizes larger than 4G go to large sections, but IMO, we should not rely on this. The specification says unlimited data size, so we have to be prepared for this. The reason I am against checking "Z" constraint is that "@SIZE" relocation returns _size_ of the object (which is only remotely connected to its address), while "Z" checks the address of the object. So, it looks to me that this check would be conceptually wrong. The right place to "fix" movabs is in a linker relaxation pass that can check the real size and change movabs to movl if the size fits 32bits. As a side note, perhaps generic option subsystem should reject overflows as e.g. -mlarge-data-threshold=5368709120 when 32bit UInt is used for option argument. The effects of overflow are surprising. Uros.
2013/11/21 Uros Bizjak <ubizjak@gmail.com>: > On Wed, Nov 20, 2013 at 5:33 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: >>>> CM_MEDIUM has unlimited data size. >>>> >>>> i386-opts.h: CM_MEDIUM, /* Assumes code fits in the low 31 >>>> bits; data unlimited. */ >>>> >>>> The x86_64_zext_immediate_operand allows _address_ to be loaded by >>>> movl. The @SIZE reloc returns object size, which is unlimited and has >>>> no connection to its address. For CM_MEDIUM, >>>> x86_64_zext_immediate_operand allows: >>>> >>>> return (ix86_cmodel == CM_SMALL >>>> || (ix86_cmodel == CM_MEDIUM >>>> && !SYMBOL_REF_FAR_ADDR_P (op))); >>> >>> Yes, but large symbols never have SYMBOL_FLAG_FAR_ADDR set. >> >> Sorry, I mean all large symbols have this flag set. > > This flag marks the fact that object size is bigger than > -mlarge-data-threshold and goes into large section. It is true that > UInt option argument currently overflows at 4G, and consequently all > sizes larger than 4G go to large sections, but IMO, we should not rely > on this. The specification says unlimited data size, so we have to be > prepared for this. > > The reason I am against checking "Z" constraint is that "@SIZE" > relocation returns _size_ of the object (which is only remotely > connected to its address), while "Z" checks the address of the object. > So, it looks to me that this check would be conceptually wrong. > > The right place to "fix" movabs is in a linker relaxation pass that > can check the real size and change movabs to movl if the size fits > 32bits. > > As a side note, perhaps generic option subsystem should reject > overflows as e.g. -mlarge-data-threshold=5368709120 when 32bit UInt is > used for option argument. The effects of overflow are surprising. Hi Uros, Thanks for explanations! I'll make a fix for the patch. BTW Pointer Bounds Checker feature does not go to 4.9 and therefore this size relocation instructions is not required in 4.9. Suppose it should go into 4.10 because doubt this feature is useful for anyone except me. This also affects all MPX instructions. With no checker in 4.9, no one actually uses them. Please tell me if I should pull them back from trunk. Thanks, Ilya > > Uros.
On Tue, Nov 26, 2013 at 12:30 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: >>>> CM_MEDIUM has unlimited data size. >>>>> >>>>> i386-opts.h: CM_MEDIUM, /* Assumes code fits in the low 31 >>>>> bits; data unlimited. */ >>>>> >>>>> The x86_64_zext_immediate_operand allows _address_ to be loaded by >>>>> movl. The @SIZE reloc returns object size, which is unlimited and has >>>>> no connection to its address. For CM_MEDIUM, >>>>> x86_64_zext_immediate_operand allows: >>>>> >>>>> return (ix86_cmodel == CM_SMALL >>>>> || (ix86_cmodel == CM_MEDIUM >>>>> && !SYMBOL_REF_FAR_ADDR_P (op))); >>>> >>>> Yes, but large symbols never have SYMBOL_FLAG_FAR_ADDR set. >>> >>> Sorry, I mean all large symbols have this flag set. >> >> This flag marks the fact that object size is bigger than >> -mlarge-data-threshold and goes into large section. It is true that >> UInt option argument currently overflows at 4G, and consequently all >> sizes larger than 4G go to large sections, but IMO, we should not rely >> on this. The specification says unlimited data size, so we have to be >> prepared for this. >> >> The reason I am against checking "Z" constraint is that "@SIZE" >> relocation returns _size_ of the object (which is only remotely >> connected to its address), while "Z" checks the address of the object. >> So, it looks to me that this check would be conceptually wrong. >> >> The right place to "fix" movabs is in a linker relaxation pass that >> can check the real size and change movabs to movl if the size fits >> 32bits. >> >> As a side note, perhaps generic option subsystem should reject >> overflows as e.g. -mlarge-data-threshold=5368709120 when 32bit UInt is >> used for option argument. The effects of overflow are surprising. > > Hi Uros, > > Thanks for explanations! I'll make a fix for the patch. > > BTW Pointer Bounds Checker feature does not go to 4.9 and therefore > this size relocation instructions is not required in 4.9. Suppose it > should go into 4.10 because doubt this feature is useful for anyone > except me. > > This also affects all MPX instructions. With no checker in 4.9, no one > actually uses them. Please tell me if I should pull them back from > trunk. The instructions in *.md are usually well isolated, so I'd suggest to remove them. Since all issues in this part ware resolved, they could be added back without problems. BR, Uros.
Index: i386.md =================================================================== --- i386.md (revision 205051) +++ i386.md (working copy) @@ -79,6 +79,7 @@ UNSPEC_PLTOFF UNSPEC_MACHOPIC_OFFSET UNSPEC_PCREL + UNSPEC_SIZEOF ;; Prologue support UNSPEC_STACK_ALLOC @@ -18458,6 +18459,30 @@ "bndstx\t{%2, %3|%3, %2}" [(set_attr "type" "mpxst")]) +(define_insn "move_size_reloc_si" + [(set (match_operand:SI 0 "register_operand" "=r") + (unspec:SI + [(match_operand:SI 1 "symbol_operand")] + UNSPEC_SIZEOF))] + "TARGET_MPX" + "mov{l}\t{%1@SIZE, %0|%0, %1@SIZE}" + [(set_attr "type" "imov")]) + +(define_insn "move_size_reloc_di" + [(set (match_operand:DI 0 "register_operand" "=r") + (unspec:DI + [(match_operand:DI 1 "symbol_operand")] + UNSPEC_SIZEOF))] + "TARGET_64BIT && TARGET_MPX" +{ + if (ix86_cmodel == CM_MEDIUM || ix86_cmodel == CM_LARGE + || ix86_cmodel == CM_MEDIUM_PIC || ix86_cmodel == CM_LARGE_PIC) + return "movabs{q}\t{%1@SIZE, %0|%0, %1@SIZE}"; + else + return "mov{l}\t{%1@SIZE, %k0|%k0, %1@SIZE}"; +} + [(set_attr "type" "imov")]) + (include "mmx.md") (include "sse.md") (include "sync.md") Index: predicates.md =================================================================== --- predicates.md (revision 205051) +++ predicates.md (working copy) @@ -119,6 +119,10 @@ (match_test "TARGET_64BIT") (match_test "REGNO (op) > BX_REG"))) +;; Return true if VALUE is a symbol reference +(define_predicate "symbol_operand" + (match_code "symbol_ref")) + ;; Return true if VALUE can be stored in a sign extended immediate field. (define_predicate "x86_64_immediate_operand" (match_code "const_int,symbol_ref,label_ref,const")