This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH/ARM][v2]PR 16722, support VLDR s/d0, =Imm
- From: Will Newton <will dot newton at linaro dot org>
- To: Jiong Wang <jiong dot wang at arm dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, "nickc at redhat dot com" <nickc at redhat dot com>
- Date: Thu, 10 Jul 2014 10:47:10 +0100
- Subject: Re: [PATCH/ARM][v2]PR 16722, support VLDR s/d0, =Imm
- Authentication-results: sourceware.org; auth=none
- References: <53A016C8 dot 2090800 at arm dot com>
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.
--
Will Newton
Toolchain Working Group, Linaro