This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH v2, i386]: Introduce x86_maybe_negate_const_int

On 02/12/2010 03:15 PM, Nathan Froyd wrote:
On Fri, Feb 12, 2010 at 02:27:05PM +0100, Uros Bizjak wrote:
Attached patch introduces x86_should_negate_const_int_p predicate in
places that negate const ints in order to generate smaller or
"cleaner" (i.e. sub XX, %reg instead of add -XX, %reg) asm code. The
predicate is also enhanced to perform checks for overflow when trying
to negate 0x80...0 to all modes.

@@ -6769,16 +6691,12 @@

gcc_assert (rtx_equal_p (operands[0], operands[1]));
- /* Make things pretty and `subl $4,%eax' rather than `addl $-4,%eax'.
- Exceptions: -128 encodes smaller than 128, so swap sign and op. */
- if ((INTVAL (operands[2]) == -128
- || (INTVAL (operands[2])> 0
- && INTVAL (operands[2]) != 128))
- /* Avoid overflows. */
- && ((INTVAL (operands[2])& ((((unsigned int) 1)<< 31) - 1))))
- return "sub{q}\t{%2, %0|%0, %2}";
- operands[2] = GEN_INT (-INTVAL (operands[2]));
- return "add{q}\t{%2, %0|%0, %2}";
+ if (x86_should_negate_const_int_p (operands[2], DImode))
+ {
+ operands[2] = GEN_INT (-INTVAL (operands[2]));
+ return "add{q}\t{%2, %0|%0, %2}";
+ }
+ return "sub{q}\t{%2, %0|%0, %2}";
It seems like every place you use x86_should_negate_const_int_p, you're
doing this pattern of if/then negate/else.  Perhaps you should go even
farther and have:

const char *
x86_addsub_with_maybe_negated_operand (rtx *loc, enum machine_mode mode,
                                        const char *addstr, const char *substr)
    /* Most of the logic from x86_should_negate_const_int_p...  */

/* Modify *LOC if necessary. */

    /* Return ADDSTR or SUBSTR as appropriate.  */

And then all the affected places just return the result of calling that

Thanks for your suggestion!

I think that your idea of conditionally modifying *LOC cleans and simplifies the code even further. OTOH, passing addstr and substr seems like an overkill to me, since we can return a boolean to signal if the negation of the operand was performed and choose between two strings (or do some other action) at the caller site.

Attached is new version of the patch (it also adds one more spot where x86_maybe_negate_const_int can be used):

2010-02-26 Uros Bizjak <>

    * config/i386/i386.c (x86_maybe_negate_const_int): New.
    (x86_output_mi_thunk): Use x86_maybe_negate_const_int.
    * config/i386/i386-protos.h: Declare x86_maybe_negate_const_int.
    * config/i386/ (*add<mode>_1, *addsi_1_zext, *addhi_1,
    *addhi_1_lea, *addqi_1, *addqi_1_lea, *addqi_1_slp, *add<mode>_2,
    *addsi_2_zext, *addhi_2, *addqi_2, *add<mode>_3, *addsi_3_zext,
    *addhi_3, *addqi_3,*add<mode>_5, *addhi_5, *addqi_5):
    Use x86_maybe_negate_const_int to output insn mnemonic.
    (*adddi_4, *addsi_4, *addhi_4, *addqi_4): Ditto.  Remove overflow
    check from instruction predicate.  Update comments.
    * config/i386/ (sync_add<mode>): Use
    x86_maybe_negate_const_int to output insn mnemonic.

Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32}. This patch will be committed to 4.6.


Attachment: p.diff.txt
Description: Text document

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]