On Tue, Sep 17, 2013 at 10:41 AM, Ilya Enkovich <[email protected]> wrote:
>> >> The x86 part looks mostly OK (I have a couple of comments bellow), but
>> >> please first get target-independent changes reviewed and committed.
>> >
>> > Do you mean I should move bound type and mode declaration into a separate
>> > patch?
>>
>> Yes, target-independent part (middle end) has to go through the
>> separate review to check if this part is OK. The target-dependent part
>> uses the infrastructure from the middle end, so it can go into the
>> code base only after target-independent parts are committed.
>
> I sent a separate patch for bound type and mode class
> (http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01268.html). Here is target
> part of the patch with fixes you mentioned. Does it look OK?
>
> Bootstrapped and checked on linux-x86_64. Still shows incorrect length
> attribute computation (described here
> http://gcc.gnu.org/ml/gcc/2013-07/msg00311.html).
Please look at the attached patch that solves length computation
problem. The patch also implements length calculation in a generic
way, as proposed earlier.
The idea is to calculate total insn length via generic "length"
attribute calculation from "length_nobnd" attribute, but iff
length_attribute is non-null. This way, we are able to decorate
bnd-prefixed instructions by "lenght_nobnd" attribute, and generic
part will automatically call ix86_bnd_prefixed_insn_p predicate with
current insn pattern. I also belive that this approach is most
flexible to decorate future patterns.
The patch adds new attribute to a couple of patterns to illustrate its usage.
Please test this approach. Modulo length calculations, improved by the
patch in this message, I have no further comments, but please repost
complete (target part) of your patch.
Uros.
Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md (revision 202953)
+++ config/i386/i386.md (working copy)
@@ -562,12 +562,19 @@
]
(const_int 1)))
+;; When this attribute is set, calculate total insn length from
+;; length_nobnd attribute, prefixed with eventual bnd prefix byte
+(define_attr "length_nobnd" "" (const_int 0))
+
;; The (bounding maximum) length of an instruction in bytes.
;; ??? fistp and frndint are in fact fldcw/{fistp,frndint}/fldcw sequences.
;; Later we may want to split them and compute proper length as for
;; other insns.
(define_attr "length" ""
- (cond [(eq_attr "type" "other,multi,fistp,frndint")
+ (cond [(eq_attr "length_nobnd" "!0")
+ (plus (symbol_ref ("ix86_bnd_prefixed_insn_p (insn)"))
+ (attr "length_nobnd"))
+ (eq_attr "type" "other,multi,fistp,frndint")
(const_int 16)
(eq_attr "type" "fcmp")
(const_int 4)
@@ -10683,7 +10690,7 @@
"%+j%C1\t%l0"
[(set_attr "type" "ibr")
(set_attr "modrm" "0")
- (set (attr "length")
+ (set (attr "length_nobnd")
(if_then_else (and (ge (minus (match_dup 0) (pc))
(const_int -126))
(lt (minus (match_dup 0) (pc))
@@ -10701,7 +10708,7 @@
"%+j%c1\t%l0"
[(set_attr "type" "ibr")
(set_attr "modrm" "0")
- (set (attr "length")
+ (set (attr "length_nobnd")
(if_then_else (and (ge (minus (match_dup 0) (pc))
(const_int -126))
(lt (minus (match_dup 0) (pc))
@@ -11623,7 +11630,7 @@
[(simple_return)]
"reload_completed"
"ret"
- [(set_attr "length" "1")
+ [(set_attr "length_nobnd" "1")
(set_attr "atom_unit" "jeu")
(set_attr "length_immediate" "0")
(set_attr "modrm" "0")])
