This is the mail archive of the binutils@sourceware.org mailing list for the binutils 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]

Re: [PATCH/ARM][v2]PR 16722, support VLDR s/d0, =Imm



On 10/07/14 10:47, Will Newton wrote:
On 17 June 2014 11:22, Jiong Wang <jiong.wang@arm.com> wrote:
This patch addressed all comments at
https://sourceware.org/ml/binutils/2014-04/msg00263.html

important updates including:

* fix 32-bit host bug.
   on 32-bit host, X_add_number field in expression is actually 4 byte, thus
8 byte
   constant will go into big number, while this cause trouble. because big
number
   is record in global variable "generic_bignum" which will be overrided by
later
   assignment.

   To solve this problem, *parse_big_immediate* is invoked *instead of
my_get_expression*,
   so 8 byte value is actually splitted and stored in "imm" and "reg" field
in inst.operands,
   "regisimm" marked as true, and the value is kept safely to the time pool
emitted.

   so what this new patch done is
     * always split 8 byte entry into two 4 byte entries in the pool.
     * matching existed 8 byte entry still work.
     * 4 byte entry could reuse half of 8 byte entry is matched.

* alignment
     * literal pool always start at 8 byte boundary.
     * 8 byte entry aligned to 8 byte boundary, padding is necessary.
     * 4 byte entry could utilize any padding existed.

* big-endian
  * test done
     * no regression on x86-64 cross arm-none-eabi.
     * no regression for check-gas/binutils/ld on chromebook native
arm-linux-gnueabihf.
     * big-endian also pass one hand-written execution test on qemu.

please review, thanks.

PR target/16722
gas/
   * config/tc-arm.c (literal_pool): New field "alignment".
   (find_or_make_literal_pool): Initialize "alignment" to 2.
   (s_ltorg): Align the pool using value of "alignment"
   (parse_big_immediate): New parameter "in_exp". Return
   parsed expression if "in_exp" is not null.
   (parse_address_main): Invoke "parse_big_immediate" for
   constant parameter.
   (add_to_lit_pool): Add one parameter 'nbytes'.
   Split 8 byte entry into two 4 byte entry.
   Add padding to align 8 byte entry to 8 byte boundary.
   (encode_arm_cp_address): Generate literal pool entry if possible.
   (move_or_literal_pool): Generate entry for vldr case.
   (enum lit_type): New enum type.
   (do_ldst): Use new enum type.
   (do_ldstv4): Likewise.
   (do_t_ldst): Likewise.
   (neon_write_immbits): Support Thumb-2 mode.
  gas/testsuite/
   * gas/arm/ldconst.s: Add test cases for symbol literal.
   * gas/arm/ldconst.d: Likewise.
   * gas/arm/vldconst.s: Add test cases for vldr.
   * gas/arm/thumb2_vpool.s: Likewise.
   * gas/arm/vldconst.d: New pattern for little-endian.
   * gas/arm/thumb2_vpool.d: Likewise.
   * gas/arm/vldconst_be.d: New pattern for big-endian.
   * gas/arm/thumb2_vpool_be.d: Likewise.
Sorry for taking so long to test this, but this breaks when building
on a 32bit platform.

   unsigned imm1;
   unsigned imm2 = 0;

   if (nbytes == 8)
     {
       imm1 = inst.operands[1].imm;
       imm2 = (inst.operands[1].regisimm ? inst.operands[1].reg
            : inst.reloc.exp.X_unsigned ? 0
            : ((int64_t)(imm1)) >> 32);

On 32bit imm1 is a 32bit type, casting it to 64bit will not give any
useful result. I also get signed/unsigned comparison errors comparing
imm1/imm2 to X_add_number.

Hi Will,

  Thanks for testing.

Actually, I've done 32-bit host test on chromebook. I'll double check this.

-- Jiong




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