Hi,

On 26 August 2017 at 18:10, Pinski, Andrew <andrew.pin...@cavium.com> 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:linaro-toolchain-boun...@lists.linaro.org] On 
> Behalf Of Kugan Vivekanandarajah
> Sent: Saturday, August 26, 2017 12:40 AM
> To: Renato Golin <renato.go...@linaro.org>
> Cc: Jim Wilson <jim.wil...@linaro.org>; hpc-sig-de...@linaro.org; Linaro 
> Toolchain <linaro-toolchain@lists.linaro.org>
> Subject: Re: [hpc-sig-devel] GCC extensions for `hcqc'
>
> Hi,
>
> On 26 August 2017 at 04:04, Renato Golin <renato.go...@linaro.org> wrote:
>> +linaro-toolchain, hoping to get more eyes into it.
>>
>> cheers,
>> --renato
>>
>> On 25 August 2017 at 17:59, Masaki Arai <masaki.a...@linaro.org> 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
>> linaro-toolchain@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/linaro-toolchain
> _______________________________________________
> linaro-toolchain mailing list
> linaro-toolchain@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-toolchain
_______________________________________________
linaro-toolchain mailing list
linaro-toolchain@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-toolchain

Reply via email to