Hi,
On 26 August 2017 at 18:10, Pinski, Andrew <[email protected]> wrote:
>> However there might be pushback from upstream maintainers as this makes the
>> structure bigger
>> by adding a field. This could have implications for memory usage of the
>> compiler.
>
> I looked into the structure, adding this field is not going to make the
> structure bigger for either ILP32 or LP64 targets. If you want, you use
> bit-fields; there is one bool already there which means you can fit 8 bits in
> the same area as currently taken up by that one.
Yes. I should have checked the mem_attrs structure. This does have at
least a byte left unlike some other tightly packed structures (gimple
and some tree structures in gcc).
Thanks,
Kugan
>> Alternatively, we maybe able to get this info from dwarf info when we
>> compile with -g ?
>
> I doubt you can. He wants to know if an instruction is a spill location.
> The location of a variable might be recorded in -g (if it was an user
> variable) but not that does present the data for all temps being spilled.
>
> I think the patch is actually a good one in general just needs some cleanup.
>
> As for these comments:
>>> For example, GCC calls `output_asm_insn' directly from the `define_insn'
>>> definition in the aarch64.md file without an insn object(`output_asm_insn'
>>> calls `output_asm_operand_names').
>>> This occurs in "*cb<optab><mode>1" and
>>> "*aarch64_fcvt<su_optab><GPF:mode><GPI:mode>2_mult".
>
> Spills in GCC will always be via the mov* patterns (they are special).
> Now really *aarch64_fcvt<su_optab><GPF:mode><GPI:mode>2_mult should be fixed
> for a different reason; it does unneeded work. The fix would be something
> like (untested):
> {
> operands[2] = GEN_INT (aarch64_fpconst_pow_of_2 (operands[2]));
> return "fcvtz<su>\t%<GPI:w>0, %<GPF:s>1, %2";
> }
>
> Thanks,
> Andrew
>
>
> -----Original Message-----
> From: linaro-toolchain [mailto:[email protected]] On
> Behalf Of Kugan Vivekanandarajah
> Sent: Saturday, August 26, 2017 12:40 AM
> To: Renato Golin <[email protected]>
> Cc: Jim Wilson <[email protected]>; [email protected]; Linaro
> Toolchain <[email protected]>
> Subject: Re: [hpc-sig-devel] GCC extensions for `hcqc'
>
> Hi,
>
> On 26 August 2017 at 04:04, Renato Golin <[email protected]> wrote:
>> +linaro-toolchain, hoping to get more eyes into it.
>>
>> cheers,
>> --renato
>>
>> On 25 August 2017 at 17:59, Masaki Arai <[email protected]> wrote:
>>> Hi,
>>>
>>> I extended GCC 7.1(or GCC 7.2) for `hcqc'.
>>> I would be grateful if you could give me a comment about whether this
>>> extension is acceptable and whether this extension should be pushed
>>> upstream.
>
> I think this is a useful info. However there might be pushback from upstream
> maintainers as this makes the structure bigger by adding a field. This could
> have implications for memory usage of the compiler.
> Alternatively, we maybe able to get this info from dwarf info when we compile
> with -g ? Jim may have some input here (cc ing him).
>
> Thanks,
> Kugan
>
>>>
>>> The extended GCC's output using the option ` -fverbose-asm' is as
>>> follows:
>>>
>>> ldr w0, [x29,48] // tmp433, j(8-byte Folded Spill)
>>> ^^^^^^^^^^^^^^^^^^^ This
>>> code shows that this instruction accesses a memory area for spill
>>> codes.
>>> I made the following changes to GCC 7.1(or GCC 7.2).
>>> The related files are under `hcqc/patch/gcc-7.1.0-add'.
>>>
>>> (1) rtl.h
>>>
>>> I added flag information to `struct mem_attrs' that means whether it
>>> is a spill memory area or not.
>>>
>>> +
>>> + /* True if the MEM is for spill. */
>>> + bool for_spill_p;
>>>
>>> Also, I added an access macro for this additional field.
>>>
>>> + /* For a MEM rtx, true if its MEM is for spill. */ #define
>>> + MEM_FOR_SPILL_P(RTX) (get_mem_attrs (RTX)->for_spill_p)
>>> +
>>>
>>> (2) emit-rtl.c
>>>
>>> I added a code to turn on flags for spill memory area in function
>>> `set_mem_attrs_for_spill'.
>>>
>>> + attrs.for_spill_p = true;
>>>
>>> (3) final.c
>>>
>>> I added code to print that information in function
>>> `output_asm_operand_names'
>>> if the memory is a spill memory area,
>>>
>>> +
>>> + if (MEM_P (op) && MEM_FOR_SPILL_P (op))
>>> + {
>>> + HOST_WIDE_INT size = MEM_SIZE (op);
>>> + fprintf (asm_out_file, " (" HOST_WIDE_INT_PRINT_DEC "-byte
>>> + Folded
>>> Spill)", size);
>>> + }
>>>
>>> The above changes are implemented similarly as Clang/LLVM.
>>> Unfortunately, it is difficult for GCC to print the above "(?-byte
>>> Folded Spill)"
>>> for memory access instructions only in the same manner as Clang/LLVM.
>>> The reason is that GCC executes the above `output_asm_operand_names'
>>> even in situations where any instruction object(insn) does not exist
>>> when outputting assembly code.
>>> For example, GCC calls `output_asm_insn' directly from the `define_insn'
>>> definition in the aarch64.md file without an insn object(`output_asm_insn'
>>> calls `output_asm_operand_names').
>>> This occurs in "*cb<optab><mode>1" and
>>> "*aarch64_fcvt<su_optab><GPF:mode><GPI:mode>2_mult".
>>>
>>> From this fact, `hcqc' extracts and accumulates memory access
>>> instructions from the assembly code with the comment "(?-byte Folded
>>> Spill)".
>>>
>>> The above extensions are commonly available on almost any architecture.
>>> Also, these extensions do not affect the execution of the resulting
>>> assembly code since additional outputs are only in comments.
>>>
>>> Best regards,
>>> --
>>> --------------------------------------
>>> Masaki Arai
>>>
>> _______________________________________________
>> linaro-toolchain mailing list
>> [email protected]
>> https://lists.linaro.org/mailman/listinfo/linaro-toolchain
> _______________________________________________
> linaro-toolchain mailing list
> [email protected]
> https://lists.linaro.org/mailman/listinfo/linaro-toolchain
_______________________________________________
linaro-toolchain mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/linaro-toolchain
