Re: [PATCH v3] gcc/config/tilegx/tilegx.c (tilegx_function_profiler): Save r10 to stack before call mcount
Firstly, sorry for replying late (During these days, I worked overtime
every workday, and have to reply in weekend).
On 10/24/16 23:27, Jeff Law wrote:
> On 10/23/2016 12:11 PM, Bernd Edlinger wrote:
>> Hi,
>>
>> I don't know much about tilegx, but
>> I think the patch should work as is.
>>
>> This is because the
>> Save r10 code is a bundle
>>
>> {
>> addi sp, sp, -8
>> st sp, r10
>> }
>>
>> which stores r10 at [sp] and subtracts 8 from sp.
>>
>> The restore r10 code is actually two bundles:
> Thanks for pointing that out! I totally missed the restore was two bundles.
>
>
>>
>> addi sp, sp, 8
>> ld r10, sp
>>
>> This just adds 8 to sp, and loads r10 from there.
> Right. And with the restore as two bundles the semantics of the save/restore
> seem consistent/correct.
>
Oh, really. Sorry that I almost forgot my history about this patch.
Originally, I sent patch v1 both with 2 bundles, but when I sent patch
v2, I let "saving r10" within a bundle for optimization (I mentioned
about it in replying patch v1 on 2016-05-31).
>>
>> I don't know how __mcount is implemented, it must
>> be some asm code, almost all functions save the
>> lr at [sp] when invoked, but I don't know if __mcount
>> does that at all, if it doesn't do that, then the
>> adjusting of sp might be unnecessary.
>>
>> The only thing that might be a problem is that
>> the stack is always adjusted in multiples of 16
>> on the tilegx platform, see tilegx.h:
>>
>> #define STACK_BOUNDARY 128
>>
>> That is counted in bits, and means 16 bytes.
>> But your patch adjusts the stack only by 8.
> Missed that. Without knowing the tile ports, I can't say with any degree of
> confidence that it's safe to only adjust by 8 bytes. Adjusting by 16 seems
> safer.
>
Oh, really! After check all the output code, "addi sp" operation are all
times of 16!! So I guess, I shall addi sp 16, too (send patch v4 for it,
if no any addition reply within a week).
>>
>> Furthermore, I don't see how the stack unwinding will work
>> with this stack adjustment when no .cfi directives
>> are emitted, but that is probably not a big problem.
> I wouldn't be surprised if the tilegx isn't the only port with this problem.
> I don't think we've ever been good about making sure the unwinders are
> correct for targets where we profile before the prologue and which emit the
> profiling bits directly rather than representing them as RTL.
>
Excuse me, I have no any idea about it (in fact, in honest, I guess, I
am still not quite familiar with gcc development in details).
At present, what I can know is: after this patch, gcc can pass various
related unwinding test (including nested functions) under qemu tilegx
linux-user (originally, I traced related insns, they should be ok).
:-)
Thanks.
--
Chen Gang (陈刚)
Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH v3] gcc/config/tilegx/tilegx.c (tilegx_function_profiler): Save r10 to stack before call mcount
On 10/20/16 06:42, Jeff Law wrote:
>> On 6/4/16 21:25, cheng...@emindsoft.com.cn wrote:
>>> From: Chen Gang
>>>
>>> r10 may also be as parameter stack pointer for the nested function, so
>>> need save it before call mcount.
>>>
>>> Also clean up code: use '!' instead of "== 0" for checking
>>> static_chain_decl and compute_total_frame_size.
>>>
>>> 2016-06-04 Chen Gang
>>>
>>> gcc/
>>> PR target/71331
>>> * config/tilegx/tilegx.c (tilegx_function_profiler): Save r10
>>> to stack before call mcount.
>>> (tilegx_can_use_return_insn_p): Clean up code.
> So if I understand the tilegx architecture correctly, you're issuing the r10
> save & sp adjustment as a bundle, and the restore & sp adjustment as a bundle.
>
> The problem is the semantics of bunding on the tilegx effectively mean that
> all source operands are read in parallel, then all outputs occur in parallel.
>
> So if we take the bundle
>
> {addi sp,sp,-8 ; st sp, r10}
>
> The address used for the st is the value of the stack pointer before the addi
> instruction.
>
> Similarly for the restore r10 bundle. The address used for the load is sp
> before adjustment.
>
> Given my understanding of the tilegx bundling semantics, that seems wrong.
>
> Jeff
>
The comments on 1st page of "TILE-Gx Instruction Set Architecture":
Individual instructions within a bundle must comply with certain register
semantics. Read-after-write (RAW) dependencies are enforced between instruction
bundles. There is no ordering within a bundle, and the numbering of pipelines
or instruction slots within a bundle is only used for convenience and does not
imply any ordering. Within an instruction bundle, it is valid to encode an
output operand that is the same as an input operand. Because there is
explicitly no implied dependency within a bundle, the semantics for this
specify that the input operands for all instructions in a bundle are read
before any of the output operands are written.
Write-after-write (WAW) semantics between two bundles are defined as: the
latest write over-writes earlier writes.
Within a bundle, WAW dependencies are forbidden. If more than one instruction
in a bundle writes to the same output operand register, unpredictable results
for any destination operand within that bundle can occur. Also, implementations
are free to signal this case as an illegal instruction. There is one exception
to this rule—multiple instructions within a bundle may legally target the zero
register. Lastly, some instructions, such as instructions that implicitly write
the link register, implicitly write registers. If an instruction implicitly
writes to a register that another instruction in the same bundle writes to,
unpredictable results can occur for any output register used by that bundle
and/or an illegal instruction interrupt can occur.
On Page 221, ld instruction is:
ld Dest, Src
On Page 251, st instruction is:
st SrcA, SrcB
So for me:
Bundle {addi sp, sp, 8; ld r10, sp} is OK, it is RAW.
Bundle {addi sp, sp, -8; st sp, r10} is OK, too, it is RAW (not WAW --
both SrcA and SrcB are input operands).
Please help check, if need the related document, please let me know.
Thanks.
--
Chen Gang (陈刚)
Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH v3] gcc/config/tilegx/tilegx.c (tilegx_function_profiler): Save r10 to stack before call mcount
Hello Maintainers:
Is this patch OK? Please help check it when you have time (at least for
me, it passes all related test and comparation).
And I shall continue to find and fix another issues about tilegx.
Thanks.
On 6/4/16 21:25, cheng...@emindsoft.com.cn wrote:
> From: Chen Gang
>
> r10 may also be as parameter stack pointer for the nested function, so
> need save it before call mcount.
>
> Also clean up code: use '!' instead of "== 0" for checking
> static_chain_decl and compute_total_frame_size.
>
> 2016-06-04 Chen Gang
>
> gcc/
> PR target/71331
> * config/tilegx/tilegx.c (tilegx_function_profiler): Save r10
> to stack before call mcount.
> (tilegx_can_use_return_insn_p): Clean up code.
> ---
> gcc/config/tilegx/tilegx.c | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/config/tilegx/tilegx.c b/gcc/config/tilegx/tilegx.c
> index 06c832c..55161bb 100644
> --- a/gcc/config/tilegx/tilegx.c
> +++ b/gcc/config/tilegx/tilegx.c
> @@ -3880,8 +3880,8 @@ bool
> tilegx_can_use_return_insn_p (void)
> {
>return (reload_completed
> - && cfun->static_chain_decl == 0
> - && compute_total_frame_size () == 0
> + && !cfun->static_chain_decl
> + && !compute_total_frame_size ()
> && tilegx_current_function_is_leaf ()
> && !crtl->profile && !df_regs_ever_live_p (TILEGX_LINK_REGNUM));
> }
> @@ -5507,6 +5507,15 @@ tilegx_function_profiler (FILE *file, int labelno
> ATTRIBUTE_UNUSED)
>fprintf (file, "\t}\n");
> }
>
> + if (cfun->static_chain_decl)
> +{
> + fprintf (file,
> +"\t{\n"
> +"\taddi\tsp, sp, -8\n"
> +"\tst\tsp, r10\n"
> +"\t}\n");
> +}
> +
>if (flag_pic)
> {
>fprintf (file,
> @@ -5524,6 +5533,13 @@ tilegx_function_profiler (FILE *file, int labelno
> ATTRIBUTE_UNUSED)
> "\t}\n", MCOUNT_NAME);
> }
>
> + if (cfun->static_chain_decl)
> +{
> + fprintf (file,
> +"\taddi\tsp, sp, 8\n"
> +"\tld\tr10, sp\n");
> +}
> +
>tilegx_in_bundle = false;
> }
>
>
--
Chen Gang (陈刚)
Managing Natural Environments is the Duty of Human Beings.
--
Chen Gang (陈刚)
Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH v2] gcc/config/tilegx/tilegx.c (tilegx_function_profiler): Save r10 to stack before call mcount
On 6/3/16 09:21, Richard Henderson wrote:
> On 06/02/2016 03:23 PM, cheng...@emindsoft.com.cn wrote:
>>fprintf (file,
>> + "\t{\n"
>> + "\taddi\tsp, sp, -8\n"
>> + "\tst\tsp, r10\n"
>> + "\t}\n"
>> "\t{\n"
>
> You need only do this if cfun->static_chain_decl is set.
>
OK, thanks, I shall send patch v3 for it, within this week end.
Thanks.
--
Chen Gang (陈刚)
Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH] gcc/config/tilegx/tilegx.c (tilegx_function_profiler): Save r10 to stack before call mcount
On 5/30/16 22:25, Chen Gang wrote:
> On 5/30/16 21:32, Chen Gang wrote:
>> On 5/30/16 03:18, Mike Stump wrote:
>>> On May 29, 2016, at 3:39 AM, cheng...@emindsoft.com.cn wrote:
>>>>
>>>> r10 may also be as parameter for the nested function, so need save it
>>>> before call mcount.
>>>
>>> mcount can have a special abi where it preserves more registers than one
>>> would otherwise expect. I'm wondering if you know what registers it saves
>>> or doesn't touch? Does this fix any bug found by running tests, or just by
>>> inspection?
>>>
Oh, I forgot bundle optimization. We can {st sp, 10; addi sp, sp, -8}.
Also I shall add PR target/71331 for the comments.
If no any additional reply within 3 days, I shall send patch v2 for it
within this week.
Thanks.
>>
>> It is about Bug71331, which I reported yesterday.
>>
>> For nested function, r10 is treated as the stack pointer for parameters,
>> mcount really save r10, but tilegx also use r10 to save lr, so cause
>> this issue ("move r10, lr; jal __mcount").
>>
>> What I have done just like gcc x86 has done ("push r10; callq mcount").
>>
>
> After this patch, nested-func-4.c can passes compiling and running under
> tilegx qemu. :-)
>
> Thanks.
>
--
Chen Gang (陈刚)
Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH] gcc/config/tilegx/tilegx.c (tilegx_function_profiler): Save r10 to stack before call mcount
On 5/30/16 21:32, Chen Gang wrote:
> On 5/30/16 03:18, Mike Stump wrote:
>> On May 29, 2016, at 3:39 AM, cheng...@emindsoft.com.cn wrote:
>>>
>>> r10 may also be as parameter for the nested function, so need save it
>>> before call mcount.
>>
>> mcount can have a special abi where it preserves more registers than one
>> would otherwise expect. I'm wondering if you know what registers it saves
>> or doesn't touch? Does this fix any bug found by running tests, or just by
>> inspection?
>>
>
> It is about Bug71331, which I reported yesterday.
>
> For nested function, r10 is treated as the stack pointer for parameters,
> mcount really save r10, but tilegx also use r10 to save lr, so cause
> this issue ("move r10, lr; jal __mcount").
>
> What I have done just like gcc x86 has done ("push r10; callq mcount").
>
After this patch, nested-func-4.c can passes compiling and running under
tilegx qemu. :-)
Thanks.
--
Chen Gang (陈刚)
Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH] gcc/config/tilegx/tilegx.c (tilegx_function_profiler): Save r10 to stack before call mcount
On 5/30/16 03:18, Mike Stump wrote:
> On May 29, 2016, at 3:39 AM, cheng...@emindsoft.com.cn wrote:
>>
>> r10 may also be as parameter for the nested function, so need save it
>> before call mcount.
>
> mcount can have a special abi where it preserves more registers than one
> would otherwise expect. I'm wondering if you know what registers it saves or
> doesn't touch? Does this fix any bug found by running tests, or just by
> inspection?
>
It is about Bug71331, which I reported yesterday.
For nested function, r10 is treated as the stack pointer for parameters,
mcount really save r10, but tilegx also use r10 to save lr, so cause
this issue ("move r10, lr; jal __mcount").
What I have done just like gcc x86 has done ("push r10; callq mcount").
Thanks.
--
Chen Gang (陈刚)
Managing Natural Environments is the Duty of Human Beings.
[PATCH] gcc/config/tilegx/tilegx.md: Compare only 32-bit values for 32-bit comparing
From 358ae2453a4b965adaf9e684220b7461f719a568 Mon Sep 17 00:00:00 2001 From: Chen Gang Date: Mon, 7 Dec 2015 21:29:20 +0800 Subject: [PATCH] gcc/config/tilegx/tilegx.md: Compare only 32-bit values for 32-bit comparing For __buildin_mul_overflow(), it will really compare only 32-bit values for 32-bit comparing. If compare 64-bit values instead of, it will cause logical issue. This fix will have low performance for 32-bit comparing (it has more instructions, also without boundling), but it is correct and fix the related issue. 2015-12-07 Chen Gang --- gcc/config/tilegx/tilegx.md | 34 ++ 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/gcc/config/tilegx/tilegx.md b/gcc/config/tilegx/tilegx.md index 944953c..b694dbd 100644 --- a/gcc/config/tilegx/tilegx.md +++ b/gcc/config/tilegx/tilegx.md @@ -1650,22 +1650,40 @@ }) -(define_insn "insn_cmpne_" - [(set (match_operand:I48MODE2 0 "register_operand" "=r") - (ne:I48MODE2 (match_operand:I48MODE 1 "reg_or_0_operand" "rO") - (match_operand:I48MODE 2 "reg_or_cint_operand" "rO")))] +(define_insn "insn_cmpne_di" + [(set (match_operand:I48MODE 0 "register_operand" "=r") + (ne:I48MODE (match_operand:DI 1 "reg_or_0_operand" "rO") + (match_operand:DI 2 "reg_or_cint_operand" "rO")))] "" "cmpne\t%0, %r1, %r2") -(define_insn "insn_cmpeq_" - [(set (match_operand:I48MODE2 0 "register_operand" "=r,r") - (eq:I48MODE2 (match_operand:I48MODE 1 "reg_or_0_operand" "%rO,rO") - (match_operand:I48MODE 2 "reg_or_cint_operand" "I,rO")))] +(define_insn "insn_cmpne_si" + [(set (match_operand:I48MODE 0 "register_operand" "=r") + (ne:I48MODE (match_operand:SI 1 "reg_or_0_operand" "rO") + (match_operand:SI 2 "reg_or_cint_operand" "rO")))] + "" + "xor\t%0, %r1, %r2; bfextu\t%0, %0, 0, 31; cmpne\t%0, %0, zero" + [(set_attr "type" "cannot_bundle")]) + +(define_insn "insn_cmpeq_di" + [(set (match_operand:I48MODE 0 "register_operand" "=r,r") + (eq:I48MODE (match_operand:DI 1 "reg_or_0_operand" "rO,rO") + (match_operand:DI 2 "reg_or_cint_operand" "I,rO")))] "" "@ cmpeqi\t%0, %r1, %2 cmpeq\t%0, %r1, %r2") +(define_insn "insn_cmpeq_si" + [(set (match_operand:I48MODE 0 "register_operand" "=r,r") + (eq:I48MODE (match_operand:SI 1 "reg_or_0_operand" "rO,rO") + (match_operand:SI 2 "reg_or_cint_operand" "I,rO")))] + "" + "@ + xori\t%0, %r1, %2; bfextu\t%0, %0, 0, 31; cmpeqi\t%0, %0, 0 + xor\t%0, %r1, %r2; bfextu\t%0, %0, 0, 31; cmpeqi\t%0, %0, 0" + [(set_attr "type" "cannot_bundle")]) + (define_insn "insn_cmplts_" [(set (match_operand:I48MODE2 0 "register_operand" "=r,r") (lt:I48MODE2 (match_operand:I48MODE 1 "reg_or_0_operand" "rO,rO") -- 1.9.3 0001-gcc-config-tilegx-tilegx.md-Compare-only-32-bit-valu.patch Description: Binary data
Re: [PATCH] gcc/fold-const.c: Correct the report warning position.
Hello all: After have a test, "gcc version 6.0.0 20151023 (experimental) (GCC)" has no this issue. And bug63510 can be closed. :-) So for me, we need not spend additional time resources on it. I shall continue to other issues in gcc or qemu. Now, I guess, my 1st priority is to rewrite tilegx qemu floating point insns within 2015-10-31. Welcome additional ideas, suggestions, and completions. Thanks. On 10/24/15 08:15, Chen Gang wrote: > > On 10/23/15 16:56, Richard Biener wrote: >> On Thu, Oct 22, 2015 at 9:46 PM, Jeff Law wrote: >>> >>> Note that the call to fold_binary from tree-ssa-sccvn.c has been removed. >>> So that hunk either needs to be removed or the change applied elsewhere. >>> > > Oh, really, it uses gimple_simplify instead of. > >>> I think passing around the location through fold-const.c is OK. >>> > > OK, thanks. > >>> I'd like to see a testcase in a form ready for inclusion into the testsuite. > > OK, thanks, I shall try. > >> >> As an additional remark - I'd like to see us not use input_location >> but always loc, > > For me, it sounds reasonable. > >> even if UNKNOWN_LOCATION. The diagnostic machinery should handle this >> correctly(?). That is, if bootstrap/testign doesn't show testsuite >> regressions because >> of this. >> > > I will try. > > > Hope I can finish trying above all within 2 days (2015-10-25). > > > Thanks. > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/fold-const.c: Correct the report warning position.
On 10/23/15 16:56, Richard Biener wrote: > On Thu, Oct 22, 2015 at 9:46 PM, Jeff Law wrote: >> >> Note that the call to fold_binary from tree-ssa-sccvn.c has been removed. >> So that hunk either needs to be removed or the change applied elsewhere. >> Oh, really, it uses gimple_simplify instead of. >> I think passing around the location through fold-const.c is OK. >> OK, thanks. >> I'd like to see a testcase in a form ready for inclusion into the testsuite. OK, thanks, I shall try. > > As an additional remark - I'd like to see us not use input_location > but always loc, For me, it sounds reasonable. > even if UNKNOWN_LOCATION. The diagnostic machinery should handle this > correctly(?). That is, if bootstrap/testign doesn't show testsuite > regressions because > of this. > I will try. Hope I can finish trying above all within 2 days (2015-10-25). Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/fold-const.c: Correct the report warning position.
Hello all: It is for bug63510, which reported by another members (not me), I guess, this patch should fix this bug. Welcome any other members' ideas, suggestions, and completions. Thanks. On 10/13/15 06:36, Chen Gang wrote: > Hello all: > > Is this patch OK? If it still needs to do anything, please let me know, > I shall try. > > Thanks. > > On 9/1/15 21:42, Chen Gang wrote: >> On 8/31/15 19:12, Richard Biener wrote: >>> On Sat, Aug 29, 2015 at 2:57 PM, Chen Gang >>> wrote: >>>> >>>> It is about bug63510: current input_location isn't precise for reporting >>>> warning. The correct location is gimple location of current statement. >>> >>> Looks ok to me. Ok if bootstrapped and tested. >>> >> >> It passes "make check". :-) >> Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/ira.c: Check !HAVE_FP_INSTEAD_INSNS when frame pointer is needed and as global register
On 10/13/15 22:56, Bernd Schmidt wrote: > On 10/13/2015 04:50 PM, Chen Gang wrote: >> OK, under the bugzilla, the maintainer treated it as expected behavior >> (not a bug). For me, we need more explanation for it (why we treat it >> as expected behavior). > > A global register is under control of the user. If the compiler uses it as a > frame pointer, it will get clobbered outside the user's control, which is > unexpected behaviour. Therefore, the code Mike quoted detects that case and > issues an error, indicating that you must use -fomit-frame-pointer if you > expect to use the frame pointer register for other purposes. > OK, thanks. > If you want an address on the stack there's __builtin_frame_address which may > or may not do what was intended. The code quoted in the bugzilla is just > invalid. > OK, thank you very much, I shall send related kernel fix patch to kernel mailing list. Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/ira.c: Check !HAVE_FP_INSTEAD_INSNS when frame pointer is needed and as global register
On 10/13/15 07:02, Mike Stump wrote:
> On Oct 12, 2015, at 3:32 PM, Chen Gang wrote:
>>
>> OK, thanks. If we really need to fix it, which target hook should I use?
>> (or do we need a new target hook?)
>
> So, the first discussion would be if it is, or is not a bug. If it isn’t,
> then there is no fix. No fix, no target hook. So far, Bernd said not a bug.
>
OK, under the bugzilla, the maintainer treated it as expected behavior
(not a bug). For me, we need more explanation for it (why we treat it
as expected behavior).
> So, I’ll note that one _can_ do this with the stack pointer, as a fixed
> register.
> When the frame pointer is fixed, one cannot do this.
>
Excuse me, I do not quite understand, could you please provide more
details?
> The code that does this is:
>
> /* Diagnose uses of the hard frame pointer when it is used as a global
>
>
> register. Often we can get away with letting the user appropriate
>
>
> the frame pointer, but we should let them know when code generation
>
>
> makes that impossible. */
> if (global_regs[HARD_FRAME_POINTER_REGNUM] && frame_pointer_needed)
> {
> tree decl = global_regs_decl[HARD_FRAME_POINTER_REGNUM];
> error_at (DECL_SOURCE_LOCATION (current_function_decl),
> "frame pointer required, but reserved");
> inform (DECL_SOURCE_LOCATION (decl), "for %qD", decl);
> }
>
> to `fix it’, one would simple remove this chunk as misguided and fix up any
> code gen issues exposed.
>
If there were not only one issues related with it, for me, what you said
sounds reasonable to me.
Thanks.
--
Chen Gang (陈刚)
Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/fold-const.c: Correct the report warning position.
Hello all: Is this patch OK? If it still needs to do anything, please let me know, I shall try. Thanks. On 9/1/15 21:42, Chen Gang wrote: > On 8/31/15 19:12, Richard Biener wrote: >> On Sat, Aug 29, 2015 at 2:57 PM, Chen Gang >> wrote: >>> >>> It is about bug63510: current input_location isn't precise for reporting >>> warning. The correct location is gimple location of current statement. >> >> Looks ok to me. Ok if bootstrapped and tested. >> > > It passes "make check". :-) > > Thanks. > -- > Chen Gang > > Open, share, and attitude like air, water, and life which God blessed > > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/ira.c: Check !HAVE_FP_INSTEAD_INSNS when frame pointer is needed and as global register
On 10/12/15 18:49, Bernd Schmidt wrote: > On 10/11/2015 05:16 PM, Chen Gang wrote: >> For some architectures (e.g. bfin), when this case occurs, they will use >> another instructions instead of frame pointer (e.g. LINK for bfin), so >> they can still generate correct output assembly code. > > What is "this case"? I don't think you have explained the problem you are > trying to solve. > It is about Bug65804. I found it when building Linux bfin kernel, and the original old version gcc can build kernel successfully. But since the git commit "e52beba PR debug/54694", it will be failed: it intends to check failure during building time. But for bfin, it has LINK insn instead of, so it is still OK, and gcc should not report failure. >> 2015-10-11 Chen Gang >> >> gcc/ >> * config.in: Add HAVE_FP_INSTEAD_INSNS. >> * configure: Check HAVE_FP_INSTEAD_INSNS to set 0 or 1. > > And of course, that should not be a configure check. If at all, use a target > hook. > OK, thanks. If we really need to fix it, which target hook should I use? (or do we need a new target hook?) Thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
[PATCH] gcc/ira.c: Check !HAVE_FP_INSTEAD_INSNS when frame pointer is needed and as global register
From fadd991f87dbd5752e9b6a2cce300dfe7cc8af25 Mon Sep 17 00:00:00 2001
From: Chen Gang
Date: Sun, 11 Oct 2015 22:59:35 +0800
Subject: [PATCH] gcc/ira.c: Check !HAVE_FP_INSTEAD_INSNS when frame pointer is
needed and as global register
For some architectures (e.g. bfin), when this case occurs, they will use
another instructions instead of frame pointer (e.g. LINK for bfin), so
they can still generate correct output assembly code.
2015-10-11 Chen Gang
gcc/
* config.in: Add HAVE_FP_INSTEAD_INSNS.
* configure: Check HAVE_FP_INSTEAD_INSNS to set 0 or 1.
* ira.c: Check !HAVE_FP_INSTEAD_INSNS when frame pointer is
needed and as global register.
---
gcc/config.in | 5 +
gcc/configure | 18 ++
gcc/ira.c | 3 ++-
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/gcc/config.in b/gcc/config.in
index 093478c..97f5957 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -1154,6 +1154,11 @@
#undef HAVE_FWRITE_UNLOCKED
#endif
+/* Define 0/1 if your machine supports frame pointer instead instructions. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_FP_INSTEAD_INSNS
+#endif
+
/* Define if your assembler supports specifying the alignment of objects
allocated using the GAS .comm command. */
diff --git a/gcc/configure b/gcc/configure
index f6ae9906..a1aa430 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -22356,6 +22356,24 @@ fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_as_flags">&5
$as_echo "$gcc_cv_as_flags">&6; }
+# Check if the target have frame pointer instead instructions
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking target frame pointer instead
instructions">&5
+$as_echo_n "checking target frame pointer instead instructions...">&6; }
+case "$cpu_type" in
+ bfin)
+ gcc_cv_fp_instead="yes"
+ ;;
+ *)
+ gcc_cv_fp_instead="no"
+ ;;
+esac
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_fp_instead">&5
+$as_echo "$gcc_cv_fp_instead">&6; }
+
+cat>>confdefs.h <<_ACEOF
+#define HAVE_FP_INSTEAD_INSNS `if test $gcc_cv_fp_instead = yes; then echo 1;
else echo 0; fi`
+_ACEOF
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for .balign and
.p2align">&5
$as_echo_n "checking assembler for .balign and .p2align... ">&6; }
if test "${gcc_cv_as_balign_and_p2align+set}" = set; then :
diff --git a/gcc/ira.c b/gcc/ira.c
index 28517c1..998d11b 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -5471,7 +5471,8 @@ do_reload (void)
register. Often we can get away with letting the user appropriate
the frame pointer, but we should let them know when code generation
makes that impossible. */
- if (global_regs[HARD_FRAME_POINTER_REGNUM] && frame_pointer_needed)
+ if (global_regs[HARD_FRAME_POINTER_REGNUM] && frame_pointer_needed
+ && !HAVE_FP_INSTEAD_INSNS)
{
tree decl = global_regs_decl[HARD_FRAME_POINTER_REGNUM];
error_at (DECL_SOURCE_LOCATION (current_function_decl),
--
1.9.3
Re: [PATCH] gcc/fold-const.c: Correct the report warning position.
On 8/31/15 19:12, Richard Biener wrote: > On Sat, Aug 29, 2015 at 2:57 PM, Chen Gang > wrote: >> >> It is about bug63510: current input_location isn't precise for reporting >> warning. The correct location is gimple location of current statement. > > Looks ok to me. Ok if bootstrapped and tested. > It passes "make check". :-) Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
[PATCH] gcc/fold-const.c: Correct the report warning position.
It is about bug63510: current input_location isn't precise for reporting
warning. The correct location is gimple location of current statement.
ChangeLog:
2015-08-29 Chen Gang
* fold-const.c (fold_overflow_warning): Call warning_at instead
of call warning.
* tree-ssa-sccvn.c (sccvn_dom_walker::before_dom_children): Call
fold_binary_loc instead of call fold_binary.
---
gcc/fold-const.c | 91
gcc/tree-ssa-sccvn.c | 2 +-
2 files changed, 58 insertions(+), 35 deletions(-)
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 1e01726..f22f070 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -299,7 +299,8 @@ fold_deferring_overflow_warnings_p (void)
overflow is undefined. */
static void
-fold_overflow_warning (const char* gmsgid, enum warn_strict_overflow_code wc)
+fold_overflow_warning (location_t loc, const char* gmsgid,
+ enum warn_strict_overflow_code wc)
{
if (fold_deferring_overflow_warnings> 0)
{
@@ -311,7 +312,8 @@ fold_overflow_warning (const char* gmsgid, enum
warn_strict_overflow_code wc)
}
}
else if (issue_strict_overflow_warning (wc))
- warning (OPT_Wstrict_overflow, gmsgid);
+ warning_at (loc == UNKNOWN_LOCATION ? input_location : loc,
+ OPT_Wstrict_overflow, gmsgid);
}
/* Return true if the built-in mathematical function specified by CODE
@@ -679,7 +681,7 @@ fold_negate_expr (location_t loc, tree t)
if (INTEGRAL_TYPE_P (type)
&& (TREE_CODE (tem) != INTEGER_CST
|| integer_onep (tem)))
- fold_overflow_warning (warnmsg, WARN_STRICT_OVERFLOW_MISC);
+ fold_overflow_warning (loc, warnmsg, WARN_STRICT_OVERFLOW_MISC);
return fold_build2_loc (loc, TREE_CODE (t), type,
TREE_OPERAND (t, 0), negate_expr (tem));
}
@@ -5143,7 +5145,7 @@ fold_range_test (location_t loc, enum tree_code code,
tree type,
in_p, low, high
{
if (strict_overflow_p)
- fold_overflow_warning (warnmsg, WARN_STRICT_OVERFLOW_COMPARISON);
+ fold_overflow_warning (loc, warnmsg, WARN_STRICT_OVERFLOW_COMPARISON);
return or_op ? invert_truthvalue_loc (loc, tem) : tem;
}
@@ -5177,7 +5179,7 @@ fold_range_test (location_t loc, enum tree_code code,
tree type,
low1, high1
{
if (strict_overflow_p)
- fold_overflow_warning (warnmsg,
+ fold_overflow_warning (loc, warnmsg,
WARN_STRICT_OVERFLOW_COMPARISON);
return build2_loc (loc, code == TRUTH_ANDIF_EXPR
? TRUTH_AND_EXPR : TRUTH_OR_EXPR,
@@ -8177,7 +8179,7 @@ maybe_canonicalize_comparison (location_t loc, enum
tree_code code, tree type,
if (t)
{
if (strict_overflow_p)
- fold_overflow_warning (warnmsg, WARN_STRICT_OVERFLOW_MAGNITUDE);
+ fold_overflow_warning (loc, warnmsg, WARN_STRICT_OVERFLOW_MAGNITUDE);
return t;
}
@@ -8188,7 +8190,7 @@ maybe_canonicalize_comparison (location_t loc, enum
tree_code code, tree type,
t = maybe_canonicalize_comparison_1 (loc, code, type, arg1, arg0,
&strict_overflow_p);
if (t && strict_overflow_p)
- fold_overflow_warning (warnmsg, WARN_STRICT_OVERFLOW_MAGNITUDE);
+ fold_overflow_warning (loc, warnmsg, WARN_STRICT_OVERFLOW_MAGNITUDE);
return t;
}
@@ -8337,9 +8339,10 @@ fold_comparison (location_t loc, enum tree_code code,
tree type,
else
{
if (!equality_code)
- fold_overflow_warning ("assuming signed overflow does not occur "
- "when changing X +- C1 cmp C2 to "
- "X cmp C2 -+ C1",
+ fold_overflow_warning (loc,
+ ("assuming signed overflow does not occur "
+ "when changing X +- C1 cmp C2 to "
+ "X cmp C2 -+ C1"),
WARN_STRICT_OVERFLOW_COMPARISON);
return fold_build2_loc (loc, code, type, variable, new_const);
}
@@ -8452,7 +8455,8 @@ fold_comparison (location_t loc, enum tree_code code,
tree type,
&& bitpos0 != bitpos1
&& (pointer_may_wrap_p (base0, offset0, bitpos0)
|| pointer_may_wrap_p (base1, offset1, bitpos1)))
- fold_overflow_warning (("assuming pointer wraparound does not "
+ fold_overflow_warning (loc,
+ ("assuming pointer wraparound does not "
[PATCH] config/bfin/bfin.c (hwloop_optimize): Recognize direct jump case and emit lsetup at loop head.
2015-07-20 Chen Gang
* config/bfin/bfin.c (hwloop_optimize): Recognize direct jump
case and emit lsetup at loop head.
---
gcc/config/bfin/bfin.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/gcc/config/bfin/bfin.c b/gcc/config/bfin/bfin.c
index a131053..1c70040 100644
--- a/gcc/config/bfin/bfin.c
+++ b/gcc/config/bfin/bfin.c
@@ -3456,7 +3456,7 @@ hwloop_optimize (hwloop_info loop)
rtx seq_end;
rtx_insn *seq;
int length;
- bool clobber0, clobber1;
+ bool clobber0, clobber1, direct_jmp = false;
if (loop->depth > MAX_LOOP_DEPTH)
{
@@ -3519,7 +3519,13 @@ hwloop_optimize (hwloop_info loop)
|| !(loop->incoming->last ()->flags & EDGE_FALLTHRU))
{
gcc_assert (JUMP_P (insn));
- insn = PREV_INSN (insn);
+ if (JUMP_LABEL (insn) != loop->start_label)
+ insn = PREV_INSN (insn);
+ else
+ {
+ direct_jmp = true;
+ insn = loop->start_label;
+ }
}
for (; insn && insn != loop->start_label; insn = NEXT_INSN (insn))
@@ -3783,7 +3789,7 @@ hwloop_optimize (hwloop_info loop)
seq = get_insns ();
end_sequence ();
- if (loop->incoming_src)
+ if (loop->incoming_src && !direct_jmp)
{
rtx_insn *prev = BB_END (loop->incoming_src);
if (vec_safe_length (loop->incoming) > 1
--
1.9.3
Re: [PATCH] config/bfin/bfin.c (hwloop_optimize): Use return false instead of gcc_assert for checking jump_insn.
On 7/7/15 06:09, Chen Gang wrote:
> On 7/6/15 20:51, Bernd Schmidt wrote:
>> On 07/03/2015 04:13 AM, Chen Gang wrote:
>>>
>>> I shall continue to analyse why 2nd lsetup optimiation has not happened.
>>> Hope I can finish within next week (2015-07-12).
>>
>> I've committed my patch after testing bfin-elf. There's no great mystery why
>> the second optimization doesn't happen: the point where it thinks it has to
>> insert the LSETUP is after the loop, and the instruction doesn't allow that.
>> Possibly we could change that - when the loop is entered at the top but not
>> through a fallthrough edge, we could make a new block ahead of it and put
>> the LSETUP in there.
>>
After trying, for me, we need notice about the jump insn to loop start
label, and emit lsetup insn to loop head instead of incoming_src end,
the related diff is below:
diff --git a/gcc/config/bfin/bfin.c b/gcc/config/bfin/bfin.c
index a131053..9ef2a9c 100644
--- a/gcc/config/bfin/bfin.c
+++ b/gcc/config/bfin/bfin.c
@@ -3416,6 +3416,7 @@ bfin_hardware_loop (void)
#define MAX_LOOP_LENGTH 2042
/* Maximum distance of the LSETUP instruction from the loop start. */
+/* #define MAX_LSETUP_DISTANCE 30 */
#define MAX_LSETUP_DISTANCE 30
/* Estimate the length of INSN conservatively. */
@@ -3456,7 +3457,7 @@ hwloop_optimize (hwloop_info loop)
rtx seq_end;
rtx_insn *seq;
int length;
- bool clobber0, clobber1;
+ bool clobber0, clobber1, direct_jmp = false;
if (loop->depth > MAX_LOOP_DEPTH)
{
@@ -3519,7 +3520,13 @@ hwloop_optimize (hwloop_info loop)
|| !(loop->incoming->last ()->flags & EDGE_FALLTHRU))
{
gcc_assert (JUMP_P (insn));
- insn = PREV_INSN (insn);
+ if (JUMP_LABEL (insn) != loop->start_label)
+ insn = PREV_INSN (insn);
+ else
+ {
+ direct_jmp = true;
+ insn = loop->start_label;
+ }
}
for (; insn && insn != loop->start_label; insn = NEXT_INSN (insn))
@@ -3783,7 +3790,7 @@ hwloop_optimize (hwloop_info loop)
seq = get_insns ();
end_sequence ();
- if (loop->incoming_src)
+ if (loop->incoming_src && !direct_jmp)
{
rtx_insn *prev = BB_END (loop->incoming_src);
if (vec_safe_length (loop->incoming) > 1
Welcome any additional ideas, suggestions and completions (and I shall
send patch for it, if no additional relply within the next week).
Thanks.
>
> OK, thanks. for me, the fix is enough for this issue. And need we add
> the related .i file to testsuite, too?
>
> And thank you for your information, I shall try to let 2nd times lsetup
> have effect in another patch, hope I can succeed :-).
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] config/bfin/bfin.c (hwloop_optimize): Use return false instead of gcc_assert for checking jump_insn.
On 7/6/15 20:51, Bernd Schmidt wrote: > On 07/03/2015 04:13 AM, Chen Gang wrote: >> On 07/01/2015 11:27 PM, Chen Gang wrote: >>> On 7/1/15 21:52, Bernd Schmidt wrote: >>>> Below is a patch. Can you test this with anything you have beyond the >>>> testsuite? >>>> >>> >>> It can fix this issue (Bug66620), let the insns standard, and can build >>> the bfin kernel with allmodconfig successfully (although for bfin kernel >>> members, they stick to allmodconfig is not a good idea for bfin kernel). >>> >>> It finished lsetup optimization for one loop, but still left the other ( >>> get the same .s as my original fix). for 2nd times in hwloop_optimize, it >>> return false. And welcome any additional ideas for it. >>> >> >> I shall continue to analyse why 2nd lsetup optimiation has not happened. >> Hope I can finish within next week (2015-07-12). > > I've committed my patch after testing bfin-elf. There's no great mystery why > the second optimization doesn't happen: the point where it thinks it has to > insert the LSETUP is after the loop, and the instruction doesn't allow that. > Possibly we could change that - when the loop is entered at the top but not > through a fallthrough edge, we could make a new block ahead of it and put the > LSETUP in there. > OK, thanks. for me, the fix is enough for this issue. And need we add the related .i file to testsuite, too? And thank you for your information, I shall try to let 2nd times lsetup have effect in another patch, hope I can succeed :-). Thanks -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] config/bfin/bfin.c (hwloop_optimize): Use return false instead of gcc_assert for checking jump_insn.
On 07/01/2015 11:27 PM, Chen Gang wrote: > On 7/1/15 21:52, Bernd Schmidt wrote: >> On 07/01/2015 03:04 AM, Chen Gang wrote: >> >>> For me, the more details are: >>> >>> - The insns have 2 loops which can be lsetup optimized. >>> >>> - After hwloop_optimize finishes 1st lsetup optimization, it generates >>> new lsetup insn which appends to jump insn in the basic block (which >>> causes the insns are not 'standard' but OK for code generation). >> >> The problem is that you can't append anything to a basic block after a jump. >> You need to create a new one. This problem doesn't usually show up since >> nothing ever looks at the basic block again, unless both directions from the >> conditional branch happen to branch to lsetup candidate loops. >> > > OK, thanks. What you said sound reasonable to me. > >> Below is a patch. Can you test this with anything you have beyond the >> testsuite? >> > > It can fix this issue (Bug66620), let the insns standard, and can build > the bfin kernel with allmodconfig successfully (although for bfin kernel > members, they stick to allmodconfig is not a good idea for bfin kernel). > > It finished lsetup optimization for one loop, but still left the other ( > get the same .s as my original fix). for 2nd times in hwloop_optimize, it > return false. And welcome any additional ideas for it. > I shall continue to analyse why 2nd lsetup optimiation has not happened. Hope I can finish within next week (2015-07-12). > For me, my original fix is incorrect: it still remains the insns in the > incorrect state (although my fix can generate the correct .s, and can > build bfin kernel with allmodconfig successfully). > > > Thanks. > -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] config/bfin/bfin.c (hwloop_optimize): Use return false instead of gcc_assert for checking jump_insn.
On 7/1/15 21:52, Bernd Schmidt wrote: > On 07/01/2015 03:04 AM, Chen Gang wrote: > >> For me, the more details are: >> >> - The insns have 2 loops which can be lsetup optimized. >> >> - After hwloop_optimize finishes 1st lsetup optimization, it generates >> new lsetup insn which appends to jump insn in the basic block (which >> causes the insns are not 'standard' but OK for code generation). > > The problem is that you can't append anything to a basic block after a jump. > You need to create a new one. This problem doesn't usually show up since > nothing ever looks at the basic block again, unless both directions from the > conditional branch happen to branch to lsetup candidate loops. > OK, thanks. What you said sound reasonable to me. > Below is a patch. Can you test this with anything you have beyond the > testsuite? > It can fix this issue (Bug66620), let the insns standard, and can build the bfin kernel with allmodconfig successfully (although for bfin kernel members, they stick to allmodconfig is not a good idea for bfin kernel). It finished lsetup optimization for one loop, but still left the other ( get the same .s as my original fix). for 2nd times in hwloop_optimize, it return false. And welcome any additional ideas for it. For me, my original fix is incorrect: it still remains the insns in the incorrect state (although my fix can generate the correct .s, and can build bfin kernel with allmodconfig successfully). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] config/bfin/bfin.c (hwloop_optimize): Use return false instead of gcc_assert for checking jump_insn.
On 06/30/2015 03:46 AM, Bernd Schmidt wrote: > On 06/28/2015 04:15 PM, Chen Gang wrote: >> For bfin looping optimization, after lsetup optimization, it can have >> the correct lsetup related insns which causes gcc_assert for jump_insn. > > I've been debugging this for a bit, and at least the explanation of the > patch is wrong - it's finding an LSETUP for a different loop. There > seems to be an inconsistency in the CFG, and it looks like it's caused > by the unusual (?) situation that both arms out of a conditional branch > lead directly to a hwloop candidate. > For me, the more details are: - The insns have 2 loops which can be lsetup optimized. - After hwloop_optimize finishes 1st lsetup optimization, it generates new lsetup insn which appends to jump insn in the basic block (which causes the insns are not 'standard' but OK for code generation). - In 2nd lsetup optimization, hwloop_optimize found the insns are not 'standard' (they are generated by hwloop_optimize itself in 1st lsetup optimization), so gcc_assert(). So hwloop_optimize can give up lsetup optimization for the 'unstandard' insns, at present, all things will be OK. If we want to try perfect, we can let hwloop_optimize can process the 'unstandard' insns for lsetup optimization. But excuse me, I am not quite familiar with gcc internal or bfin (so I am not suitable for it). > So, not OK until further investigation I think. > If necessary (what I said above is incorrect), I shall continue to analyse. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
[PATCH] config/bfin/bfin.c (hwloop_optimize): Use return false instead of gcc_assert for checking jump_insn.
For bfin looping optimization, after lsetup optimization, it can have
the correct lsetup related insns which causes gcc_assert for jump_insn.
The related bug is Bug 66620.
2015-06-28 Chen Gang
* config/bfin/bfin.c (hwloop_optimize): Use return false instead
of gcc_assert for checking jump_insn.
---
gcc/config/bfin/bfin.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/gcc/config/bfin/bfin.c b/gcc/config/bfin/bfin.c
index 3b4b54e..91866dd 100644
--- a/gcc/config/bfin/bfin.c
+++ b/gcc/config/bfin/bfin.c
@@ -3520,7 +3520,13 @@ hwloop_optimize (hwloop_info loop)
if (vec_safe_length (loop->incoming) > 1
|| !(loop->incoming->last ()->flags & EDGE_FALLTHRU))
{
- gcc_assert (JUMP_P (insn));
+ if (!JUMP_P (insn))
+ {
+ if (dump_file)
+ fprintf (dump_file, ";; loop %d lsetup may already inserted\n",
+loop->loop_no);
+ return false;
+ }
insn = PREV_INSN (insn);
}
--
1.9.3
Re: [PATCH] config/bfin/bfin.c (hwloop_optimize): Set JUMP_LABEL() after emit jump_insn
On 6/24/15 12:25, Jeff Law wrote: > On 06/20/2015 04:48 AM, Chen Gang wrote: >> JUMP_LABLE() must be defined after optimization completed. In this case, >> it is doing optimization, and is almost finished, so it is no chances to >> set JUMP_LABLE() next. The related issue is Bug 65803. >> >> 2015-06-20 Chen Gang >> >> * config/bfin/bfin.c (hwloop_optimize): Set JUMP_LABEL() after >> emit jump_insn. > Thanks. I've reduced the testcase from pr65803 and committed the changes to > the trunk along with the reduced testcase. > > I tested the bfin port lightly -- just confirmed that it'd build newlib as a > sanity test. > > Actual committed patch is attached for archival purposes. > OK, thanks. I shall continue to try another bfin bugs (which I found during building Linux kernel with allmodconfig). I shall try to finish one bug within this month (2015-06-30). After finish bfin bugs (may be several bugs left, now), I shall try tilegx testsuite with qemu (I almost finish qemu tilegx linux-user with much help by qemu members). I shall try to start within next month. And sorry for my disappearing several months: - I had to spend more time resources on qemu tilegx (for qemu tilegx, I had already delayed too much to bear). - I am not quite familiar with gcc internal contents, so if I do not spend enough time resources on an issue, I will probably send spams ( may be even worse: hiding the issues instead of solving it). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
[PATCH] config/bfin/bfin.c (hwloop_optimize): Set JUMP_LABEL() after emit jump_insn
JUMP_LABLE() must be defined after optimization completed. In this case,
it is doing optimization, and is almost finished, so it is no chances to
set JUMP_LABLE() next. The related issue is Bug 65803.
2015-06-20 Chen Gang
* config/bfin/bfin.c (hwloop_optimize): Set JUMP_LABEL() after
emit jump_insn.
---
gcc/config/bfin/bfin.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gcc/config/bfin/bfin.c b/gcc/config/bfin/bfin.c
index 3b4b54e..23c2d51 100644
--- a/gcc/config/bfin/bfin.c
+++ b/gcc/config/bfin/bfin.c
@@ -3775,7 +3775,10 @@ hwloop_optimize (hwloop_info loop)
}
else
{
- emit_jump_insn (gen_jump (label));
+ rtx_insn * ret = emit_jump_insn (gen_jump (label));
+
+ JUMP_LABEL(ret) = label;
+ LABEL_NUSES (label)++;
seq_end = emit_barrier ();
}
}
--
1.9.3
[PATCH] gcc/config/c6x/c6x.md (mvilc): Add "SI" for "unspec".
The related warning: build/genrecog ../../gcc-c6x/gcc/common.md ../../gcc-c6x/gcc/config/c6x/c6x.md \ insn-conditions.md > tmp-recog.c [...] ../../gcc-c6x/gcc/config/c6x/c6x.md:1443: warning: source missing a mode? [...] 2015-03-28 Chen Gang * config/c6x/c6x.md (mvilc): Add "SI" for "unspec". --- gcc/config/c6x/c6x.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/config/c6x/c6x.md b/gcc/config/c6x/c6x.md index e957eca..78c9bd4 100644 --- a/gcc/config/c6x/c6x.md +++ b/gcc/config/c6x/c6x.md @@ -1442,7 +1442,8 @@ (define_insn "mvilc" [(set (reg:SI REG_ILC) - (unspec [(match_operand:SI 0 "register_operand" "a,b")] UNSPEC_MVILC))] + (unspec:SI [(match_operand:SI 0 "register_operand" "a,b")] + UNSPEC_MVILC))] "TARGET_INSNS_64PLUS" "%|%.\\tmvc\\t%$\\t%0, ILC" [(set_attr "predicable" "no") -- 1.9.3
Re: [PATCH] gcc/config/c6x/c6x.md: Remove "clobber (match_scratch ...)" in "movmisalign_store".
On 3/28/15 09:32, Chen Gang wrote: > On 3/27/15 21:03, Bernd Schmidt wrote: >> On 03/27/2015 01:05 AM, Chen Gang wrote: >>> For misalignment memory access, c6x gcc will cause issue, so need remove >>> "clobber (match_scratch ...)" which will be symmetric with "movmisalign >>> _load", then pass compiling and generate correct assembly code. >>> >> >>> * config/c6x/c6x.md (movmisalign_store): Remove "clobber >>> (match_scratch ...)". >> >> No, that just will make the compiler confuse loads and stores. I've >> committed the following to fix it (I thought I'd done so a year ago, but >> probably it was one of those commit against an out-of-date tree situations >> and it didn't go through). >> > > OK, thanks. Your patch is OK to me. And I shall try to find another c6x > patches within this month (2015-03-31). > I have reported the related bug 65510, please continue for it, if I need to continue, please let me know. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
[PATCH] gcc/config/c6x/c6x.h: Handle default case for switch statement in TARGET_CPU_CPP_BUILTINS().
The related warning:
g++ -c -DIN_GCC_FRONTEND -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE
-fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing
-Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual
-pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings
-fno-common -DHAVE_CONFIG_H -I. -Ic-family -I../../gcc-c6x/gcc
-I../../gcc-c6x/gcc/c-family -I../../gcc-c6x/gcc/../include
-I../../gcc-c6x/gcc/../libcpp/include -I../../gcc-c6x/gcc/../libdecnumber
-I../../gcc-c6x/gcc/../libdecnumber/dpd -I../libdecnumber
-I../../gcc-c6x/gcc/../libbacktrace -o c-family/c-cppbuiltin.o -MT
c-family/c-cppbuiltin.o -MMD -MP -MF c-family/.deps/c-cppbuiltin.TPo
../../gcc-c6x/gcc/c-family/c-cppbuiltin.c
In file included from ./tm.h:16:0,
from ../../gcc-c6x/gcc/c-family/c-cppbuiltin.c:23:
../../gcc-c6x/gcc/c-family/c-cppbuiltin.c: In function ‘void
c_cpp_builtins(cpp_reader*)’:
../../gcc-c6x/gcc/config/c6x/c6x.h:85:14: warning: enumeration value
‘unk_isa’ not handled in switch [-Wswitch]
switch (c6x_arch)\
^../../gcc-c6x/gcc/c-family/c-cppbuiltin.c:1243:3: note: in
expansion of macro ‘TARGET_CPU_CPP_BUILTINS’
TARGET_CPU_CPP_BUILTINS ();
^
2015-03-28 Chen Gang
* config/c6x/c6x.h (TARGET_CPU_CPP_BUILTINS): Handle default
case for switch statement.
---
gcc/config/c6x/c6x.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gcc/config/c6x/c6x.h b/gcc/config/c6x/c6x.h
index 58a7ac6..b4810be 100644
--- a/gcc/config/c6x/c6x.h
+++ b/gcc/config/c6x/c6x.h
@@ -109,6 +109,9 @@ extern c6x_cpu_t c6x_arch;
builtin_define ("_TMS320C6400_PLUS"); \
builtin_define ("_TMS320C6400"); \
break;\
+ \
+ default:\
+ break;\
} \
} while (0)
--
1.9.3
Re: [PATCH] gcc/config/c6x/c6x.md: Remove "clobber (match_scratch ...)" in "movmisalign_store".
On 3/27/15 21:03, Bernd Schmidt wrote: > On 03/27/2015 01:05 AM, Chen Gang wrote: >> For misalignment memory access, c6x gcc will cause issue, so need remove >> "clobber (match_scratch ...)" which will be symmetric with "movmisalign >> _load", then pass compiling and generate correct assembly code. >> > >> * config/c6x/c6x.md (movmisalign_store): Remove "clobber >> (match_scratch ...)". > > No, that just will make the compiler confuse loads and stores. I've committed > the following to fix it (I thought I'd done so a year ago, but probably it > was one of those commit against an out-of-date tree situations and it didn't > go through). > OK, thanks. Your patch is OK to me. And I shall try to find another c6x patches within this month (2015-03-31). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/config/c6x/c6x.md: Remove "clobber (match_scratch ...)" in "movmisalign_store".
On 3/27/15 08:05, Chen Gang wrote:
> For misalignment memory access, c6x gcc will cause issue, so need remove
> "clobber (match_scratch ...)" which will be symmetric with "movmisalign
> _load", then pass compiling and generate correct assembly code.
>
> The related issue:
>
> [root@localhost c6x]# cat test.i
> int oxu_driver_init(void)
> {
>*(volatile unsigned int *)(-1) = *(unsigned int *)(-1);
Oh, sorry, I use 11 instead of -1 for generating the assembly code (-1
and 11 lead the same result).
[...]
> oxu_driver_init:
> mvk .d1 11, A4
> ldnw.d1t1 *A4, A3
> nop 4
> stnw.d1t1 A3, *A4
> ret .s2 B3
> nop 5
> .size oxu_driver_init, .-oxu_driver_init
> .ident "GCC: (GNU) 5.0.0 20150321 (experimental)"
>
[...]
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
[PATCH] gcc/config/c6x/c6x.md: Remove "clobber (match_scratch ...)" in "movmisalign_store".
For misalignment memory access, c6x gcc will cause issue, so need remove
"clobber (match_scratch ...)" which will be symmetric with "movmisalign
_load", then pass compiling and generate correct assembly code.
The related issue:
[root@localhost c6x]# cat test.i
int oxu_driver_init(void)
{
*(volatile unsigned int *)(-1) = *(unsigned int *)(-1);
}
[root@localhost c6x]#
/upstream/release-c6x/libexec/gcc/tic6x-gchen-elf/5.0.0/cc1 -Os test.i
oxu_driver_init
test.i: In function 'oxu_driver_init':
test.i:4:1: error: unrecognizable insn:
}
^
(insn 9 8 12 2 (set (mem/v:SI (reg/f:SI 77) [1 MEM[(volatile unsigned int
*)4294967295B]+0 S4 A8])
(unspec:SI [
(reg:SI 73 [ D.1542 ])
] UNSPEC_MISALIGNED_ACCESS)) test.i:3 -1
(nil))
test.i:4:1: internal compiler error: in extract_insn, at recog.c:2343
0x9f8645 _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
../../gcc-c6x/gcc/rtl-error.c:110
0x9f8679 _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
../../gcc-c6x/gcc/rtl-error.c:118
0x9ce127 extract_insn(rtx_insn*)
../../gcc-c6x/gcc/recog.c:2343
0x7c759b instantiate_virtual_regs_in_insn
../../gcc-c6x/gcc/function.c:1598
0x7c759b instantiate_virtual_regs
../../gcc-c6x/gcc/function.c:1966
0x7c759b execute
../../gcc-c6x/gcc/function.c:2015
Please submit a full bug report,
Related output after fix (I guess it is OK, please check):
.file "test.i"
.c6xabi_attribute Tag_ABI_array_object_alignment, 0
.c6xabi_attribute Tag_ABI_array_object_align_expected, 0
.c6xabi_attribute Tag_ABI_stack_align_needed, 0
.c6xabi_attribute Tag_ABI_stack_align_preserved, 0
.c6xabi_attribute Tag_ABI_conformance, "1.0"
.text;
.align 2
.global oxu_driver_init
.type oxu_driver_init, @function
oxu_driver_init:
mvk .d1 11, A4
ldnw.d1t1 *A4, A3
nop 4
stnw.d1t1 A3, *A4
ret .s2 B3
nop 5
.size oxu_driver_init, .-oxu_driver_init
.ident "GCC: (GNU) 5.0.0 20150321 (experimental)"
2015-03-27 Chen Gang
* config/c6x/c6x.md (movmisalign_store): Remove "clobber
(match_scratch ...)".
---
gcc/config/c6x/c6x.md | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/gcc/config/c6x/c6x.md b/gcc/config/c6x/c6x.md
index 892aac4..6a3157f 100644
--- a/gcc/config/c6x/c6x.md
+++ b/gcc/config/c6x/c6x.md
@@ -785,8 +785,7 @@
(define_insn_and_split "movmisalign_store"
[(set (match_operand:SIDIVM 0 "memory_operand" "=W,Q,T,Q,T")
(unspec:SIDIVM [(match_operand:SIDIVM 1 "register_operand" "r,a,b,b,a")]
- UNSPEC_MISALIGNED_ACCESS))
- (clobber (match_scratch:SI 2 "=r,X,X,X,X"))]
+ UNSPEC_MISALIGNED_ACCESS))]
"TARGET_INSNS_64"
"@
#
--
1.9.3
Re: [PATCH] gcc/genrecog.c: Check matching constraint in MATCH_OPERAND.
Hello Maintainers:
Please help check this patch when you have time.
I have to leave Sunrus, the mail address (gang.c...@sunrus.com.cn) will
be closed soon (Sunrus will be closed soon because of money, I guess).
I change my new email address (xili_gchen_5...@hotmail.com) to continue
communicating. gang.chen.5...@gmail.com is still have effect, but it is
not stable (gmail in China is not stable).
I apologize for inconvenience.
Thanks.
On 2/27/15 08:37, Chen Gang S wrote:
> On 02/26/2015 08:13 PM, Chen Gang S wrote:
>> On 02/26/2015 04:04 PM, Chen Gang S wrote:
>>> If check fails, genrecog needs to stop and report error, so can let the
>>> issue be found in time. The implementation is referenced from "gcc/doc/
>>> md.log":
>>>
>>> [...]
>>>
>>> whitespace
>>>Whitespace characters are ignored and can be inserted at any
>>>position except the first. This enables each alternative for
>>>different operands to be visually aligned in the machine
>>>description even if they have different number of constraints and
>>>modifiers.
>>>
>>> [...]
>>>
>>> '0', '1', '2', ... '9'
>>>[...]
>>>If a digit is used together with letters within the same
>>>alternative, the digit should come last.
>>>
>>
>> Oh, I guess, I misunderstood the contents above. e.g. "Up3" which
>> defined in aarch64 is not "matching constraint", I should skip it.
>>
>
> If I really misunderstood, for me, the related patch should be:
>
> diff --git a/gcc/genrecog.c b/gcc/genrecog.c
> index 81a0e79..9367d74 100644
> --- a/gcc/genrecog.c
> +++ b/gcc/genrecog.c
> @@ -503,7 +503,8 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int
> set_code)
>
> if (code == MATCH_OPERAND)
> {
> - const char constraints0 = XSTR (pattern, 2)[0];
> + const char *constraints = XSTR (pattern, 2);
> + const char constraints0 = constraints[0];
>
> if (!constraints_supported_in_insn_p (insn))
> {
> @@ -537,6 +538,33 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int
> set_code)
> "operand %d missing output reload",
> XINT (pattern, 0));
> }
> +
> + /* For matching constraint in MATCH_OPERAND, the digit must be a
> +smaller number than the number of the operand that uses it in the
> +constraint. */
> + while (1)
> + {
> + while (constraints[0]
> +&& (constraints[0] == ' ' || constraints[0] == ','))
> + constraints++;
> + if (!constraints[0])
> + break;
> +
> + if (constraints[0] >= '0' && constraints[0] <= '9')
> + {
> + int val;
> +
> + sscanf (constraints, "%d", &val);
> + if (val >= XINT (pattern, 0))
> + error_with_line (pattern_lineno,
> +"constraint digit %d is not smaller than"
> +" operand %d",
> +val, XINT (pattern, 0));
> + }
> +
> + while (constraints[0] && constraints[0] != ',')
> + constraints++;
> + }
> }
>
> /* Allowing non-lvalues in destinations -- particularly CONST_INT --
>
>
> If necessary (I guess, it is), I shall send patch v2 for it.
>
> Thanks.
>
>>>This number is allowed to be more than a single digit. If multiple
>>>digits are encountered consecutively, they are interpreted as a
>>>single decimal integer. [...]
>>>
>>>[...]
>>>
>>>[...] Moreover, the digit must be a
>>>smaller number than the number of the operand that uses it in the
>>>constraint.
>>>
>>> [...]
>>>
>>> The overall constraint for an operand is made from the letters for this
>>> operand from the first alternative, a comma, the letters for this
>>> operand from the second alternative, a comma, and so on until the last
>>> alternative.
>>>
>>> [...]
>>>
>>> The patch passes test:
>>>
>>> - genrecog
Re: [PATCH] gcc/genrecog.c: Check matching constraint in MATCH_OPERAND.
On 02/26/2015 08:13 PM, Chen Gang S wrote:
> On 02/26/2015 04:04 PM, Chen Gang S wrote:
>> If check fails, genrecog needs to stop and report error, so can let the
>> issue be found in time. The implementation is referenced from "gcc/doc/
>> md.log":
>>
>> [...]
>>
>> whitespace
>>Whitespace characters are ignored and can be inserted at any
>>position except the first. This enables each alternative for
>>different operands to be visually aligned in the machine
>>description even if they have different number of constraints and
>>modifiers.
>>
>> [...]
>>
>> '0', '1', '2', ... '9'
>>[...]
>>If a digit is used together with letters within the same
>>alternative, the digit should come last.
>>
>
> Oh, I guess, I misunderstood the contents above. e.g. "Up3" which
> defined in aarch64 is not "matching constraint", I should skip it.
>
If I really misunderstood, for me, the related patch should be:
diff --git a/gcc/genrecog.c b/gcc/genrecog.c
index 81a0e79..9367d74 100644
--- a/gcc/genrecog.c
+++ b/gcc/genrecog.c
@@ -503,7 +503,8 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int
set_code)
if (code == MATCH_OPERAND)
{
- const char constraints0 = XSTR (pattern, 2)[0];
+ const char *constraints = XSTR (pattern, 2);
+ const char constraints0 = constraints[0];
if (!constraints_supported_in_insn_p (insn))
{
@@ -537,6 +538,33 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int
set_code)
"operand %d missing output reload",
XINT (pattern, 0));
}
+
+ /* For matching constraint in MATCH_OPERAND, the digit must be a
+ smaller number than the number of the operand that uses it in the
+ constraint. */
+ while (1)
+ {
+ while (constraints[0]
+ && (constraints[0] == ' ' || constraints[0] == ','))
+ constraints++;
+ if (!constraints[0])
+ break;
+
+ if (constraints[0] >= '0' && constraints[0] <= '9')
+ {
+ int val;
+
+ sscanf (constraints, "%d", &val);
+ if (val >= XINT (pattern, 0))
+ error_with_line (pattern_lineno,
+ "constraint digit %d is not smaller than"
+ " operand %d",
+ val, XINT (pattern, 0));
+ }
+
+ while (constraints[0] && constraints[0] != ',')
+ constraints++;
+ }
}
/* Allowing non-lvalues in destinations -- particularly CONST_INT --
If necessary (I guess, it is), I shall send patch v2 for it.
Thanks.
>>This number is allowed to be more than a single digit. If multiple
>>digits are encountered consecutively, they are interpreted as a
>>single decimal integer. [...]
>>
>>[...]
>>
>>[...] Moreover, the digit must be a
>>smaller number than the number of the operand that uses it in the
>>constraint.
>>
>> [...]
>>
>> The overall constraint for an operand is made from the letters for this
>> operand from the first alternative, a comma, the letters for this
>> operand from the second alternative, a comma, and so on until the last
>> alternative.
>>
>> [...]
>>
>> The patch passes test:
>>
>> - genrecog can report the related issue when cross compiling xtensa.
>>And after the related xtensa issue is fixed, genrecog will not report
>>error, either.
>>
>> - For x86_64-unknown-linux-gnu building all-gcc, it is OK, too.
>>
>>
>> 2015-02-26 Chen Gang
>>
>> * genrecog.c (validate_pattern): Check matching constraint in
>> MATCH_OPERAND and use 'opnu' for all 'XINT (pattern, 0)'.
>> ---
>> gcc/genrecog.c | 39 +++
>> 1 file changed, 35 insertions(+), 4 deletions(-)
>>
>> diff --git a/gcc/genrecog.c b/gcc/genrecog.c
>> index 81a0e79..4b508e8 100644
>> --- a/gcc/genrecog.c
>> +++ b/gcc/genrecog.c
>> @@ -503,7 +503,9 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int
>> set_code)
>>
>>
Re: [PATCH] gcc/genrecog.c: Check matching constraint in MATCH_OPERAND.
On 02/26/2015 04:04 PM, Chen Gang S wrote:
> If check fails, genrecog needs to stop and report error, so can let the
> issue be found in time. The implementation is referenced from "gcc/doc/
> md.log":
>
> [...]
>
> whitespace
>Whitespace characters are ignored and can be inserted at any
>position except the first. This enables each alternative for
>different operands to be visually aligned in the machine
>description even if they have different number of constraints and
>modifiers.
>
> [...]
>
> '0', '1', '2', ... '9'
>[...]
>If a digit is used together with letters within the same
>alternative, the digit should come last.
>
Oh, I guess, I misunderstood the contents above. e.g. "Up3" which
defined in aarch64 is not "matching constraint", I should skip it.
>This number is allowed to be more than a single digit. If multiple
>digits are encountered consecutively, they are interpreted as a
>single decimal integer. [...]
>
>[...]
>
>[...] Moreover, the digit must be a
>smaller number than the number of the operand that uses it in the
>constraint.
>
> [...]
>
> The overall constraint for an operand is made from the letters for this
> operand from the first alternative, a comma, the letters for this
> operand from the second alternative, a comma, and so on until the last
> alternative.
>
> [...]
>
> The patch passes test:
>
> - genrecog can report the related issue when cross compiling xtensa.
>And after the related xtensa issue is fixed, genrecog will not report
>error, either.
>
> - For x86_64-unknown-linux-gnu building all-gcc, it is OK, too.
>
>
> 2015-02-26 Chen Gang
>
> * genrecog.c (validate_pattern): Check matching constraint in
> MATCH_OPERAND and use 'opnu' for all 'XINT (pattern, 0)'.
> ---
> gcc/genrecog.c | 39 +++
> 1 file changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/genrecog.c b/gcc/genrecog.c
> index 81a0e79..4b508e8 100644
> --- a/gcc/genrecog.c
> +++ b/gcc/genrecog.c
> @@ -503,7 +503,9 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int
> set_code)
>
> if (code == MATCH_OPERAND)
> {
> - const char constraints0 = XSTR (pattern, 2)[0];
> + int opnu = XINT (pattern, 0);
> + const char *constraints = XSTR (pattern, 2);
> + const char constraints0 = constraints[0];
>
> if (!constraints_supported_in_insn_p (insn))
> {
> @@ -525,17 +527,46 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int
> set_code)
> /* If we've only got an output reload for this operand,
> we'd better have a matching input operand. */
> else if (constraints0 == '='
> - && find_matching_operand (insn, XINT (pattern, 0)))
> + && find_matching_operand (insn, opnu))
> ;
> else
> error_with_line (pattern_lineno,
> "operand %d missing in-out reload",
> -XINT (pattern, 0));
> +opnu);
> }
> else if (constraints0 != '=' && constraints0 != '+')
> error_with_line (pattern_lineno,
> "operand %d missing output reload",
> -XINT (pattern, 0));
> +opnu);
> + }
> +
> + /* For matching constraint in MATCH_OPERAND, the digit must be a
> +smaller number than the number of the operand that uses it in the
> +constraint. */
> + while (1)
> + {
> + int val;
> +
> + while (constraints[0]
> +&& (constraints[0] < '0' || constraints[0] > '9'))
> + constraints++;
> + if (!constraints[0])
> + break;
> +
> + sscanf (constraints, "%d", &val);
> + while ((constraints[0] >= '0' && constraints[0] <= '9'))
> + constraints++;
> +
> + while (constraints[0] == ' ')
> + constraints++;
> + if (constraints[0] && constraints[0] != ',')
> + continue;
> +
> + if (val >= opnu)
> + error_with_line (pattern_lineno,
> +"constraint digit %d is not smaller than"
> +" operand %d",
> +val, opnu);
> }
> }
>
>
--
Open, share, and attitude like air, water, and life which God blessed.
[PATCH] gcc/genrecog.c: Check matching constraint in MATCH_OPERAND.
If check fails, genrecog needs to stop and report error, so can let the
issue be found in time. The implementation is referenced from "gcc/doc/
md.log":
[...]
whitespace
Whitespace characters are ignored and can be inserted at any
position except the first. This enables each alternative for
different operands to be visually aligned in the machine
description even if they have different number of constraints and
modifiers.
[...]
'0', '1', '2', ... '9'
[...]
If a digit is used together with letters within the same
alternative, the digit should come last.
This number is allowed to be more than a single digit. If multiple
digits are encountered consecutively, they are interpreted as a
single decimal integer. [...]
[...]
[...] Moreover, the digit must be a
smaller number than the number of the operand that uses it in the
constraint.
[...]
The overall constraint for an operand is made from the letters for this
operand from the first alternative, a comma, the letters for this
operand from the second alternative, a comma, and so on until the last
alternative.
[...]
The patch passes test:
- genrecog can report the related issue when cross compiling xtensa.
And after the related xtensa issue is fixed, genrecog will not report
error, either.
- For x86_64-unknown-linux-gnu building all-gcc, it is OK, too.
2015-02-26 Chen Gang
* genrecog.c (validate_pattern): Check matching constraint in
MATCH_OPERAND and use 'opnu' for all 'XINT (pattern, 0)'.
---
gcc/genrecog.c | 39 +++
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/gcc/genrecog.c b/gcc/genrecog.c
index 81a0e79..4b508e8 100644
--- a/gcc/genrecog.c
+++ b/gcc/genrecog.c
@@ -503,7 +503,9 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int
set_code)
if (code == MATCH_OPERAND)
{
- const char constraints0 = XSTR (pattern, 2)[0];
+ int opnu = XINT (pattern, 0);
+ const char *constraints = XSTR (pattern, 2);
+ const char constraints0 = constraints[0];
if (!constraints_supported_in_insn_p (insn))
{
@@ -525,17 +527,46 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int
set_code)
/* If we've only got an output reload for this operand,
we'd better have a matching input operand. */
else if (constraints0 == '='
-&& find_matching_operand (insn, XINT (pattern, 0)))
+&& find_matching_operand (insn, opnu))
;
else
error_with_line (pattern_lineno,
"operand %d missing in-out reload",
- XINT (pattern, 0));
+ opnu);
}
else if (constraints0 != '=' && constraints0 != '+')
error_with_line (pattern_lineno,
"operand %d missing output reload",
- XINT (pattern, 0));
+ opnu);
+ }
+
+ /* For matching constraint in MATCH_OPERAND, the digit must be a
+ smaller number than the number of the operand that uses it in the
+ constraint. */
+ while (1)
+ {
+ int val;
+
+ while (constraints[0]
+ && (constraints[0] < '0' || constraints[0] > '9'))
+ constraints++;
+ if (!constraints[0])
+ break;
+
+ sscanf (constraints, "%d", &val);
+ while ((constraints[0] >= '0' && constraints[0] <= '9'))
+ constraints++;
+
+ while (constraints[0] == ' ')
+ constraints++;
+ if (constraints[0] && constraints[0] != ',')
+ continue;
+
+ if (val >= opnu)
+ error_with_line (pattern_lineno,
+ "constraint digit %d is not smaller than"
+ " operand %d",
+ val, opnu);
}
}
--
1.9.3
Re: [PATCH] gcc/reload.c: Initialize several arrays before use them in find_reloads()
On 2/25/15 07:36, augustine.sterl...@gmail.com wrote: > On Tue, Feb 24, 2015 at 2:05 PM, Max Filippov wrote: >> Hi, >> >> On Tue, Feb 24, 2015 at 6:54 PM, Jeff Law wrote: >>> >>> You can tackle them in any order you wish. However, I suspect fixing the >>> xtensa backend may be easier. I don't have any good way to test xtensa, but >>> something like the attached patch for the xtensa port should be sufficient. >> >> I can confirm that this patch fixes the issue. I haven't got a segfault >> as was reported in bugzilla, but I got some 'conditional jump or move >> depends on uninitialised value' under valgrind, originated from >> find_reloads, that the patch fixed. And with it the testcase from >> bugzilla actually produces code with loop instruction. > > Max, the patch doesn't fix the underlying issue, it just hides the bad > memory reference by initializing what we were already accessing. > > See Jeff Law's message earlier in the thread. > So we need genrecog to find the underlying issue in time, and report error (stop building gcc). And also thank Max very much for his warmhearted response in every where (kernel, qemu, and gcc). I am sure your response will give much help to me. :-) Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/reload.c: Initialize several arrays before use them in find_reloads()
On 2/25/15 03:22, Jeff Law wrote: > On 02/24/15 10:29, Chen Gang S wrote: >> >> So for me, I shall let genrecog report error instead of warning when it >> find this issue, next. > Yes, I think an error would be appropriate here. That way nobody has to > debug same issue you just did in find_reloads. > OK, thanks, I shall try to send the related patch within this month. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/reload.c: Initialize several arrays before use them in find_reloads()
On 2/25/15 01:03, augustine.sterl...@gmail.com wrote: > On Tue, Feb 24, 2015 at 9:05 AM, Chen Gang S wrote: >> >> - After this patch, it can continue compiling, but can we be sure that >>it always generates correct code for execution? > > After this patch, it should be no worse than it was--lucky. > > Someone is working on fixing the underlying xtensa pattern in the > short term. So I would continue whether or not the generated code is > correct. > > Another short-term option would be to use an xtensa config that > doesn't have zero-over-head loops. > OK, thank you for your response. So for me, I shall let genrecog report error instead of warning when it find this issue, next. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/reload.c: Initialize several arrays before use them in find_reloads()
On 2/24/15 23:54, Jeff Law wrote: > On 02/23/15 18:33, Chen Gang S wrote: >> OK, thanks. I shall try to finish within this month (although I am >> not quite sure whether I can finish on time). > No problem. Any contribution you can make in this area is appreciated. > OK, thanks. >> OK, thanks. May I firstly finish it before fixing xtensa pattern? I >> guess it is more easier than fixing xtensa patten. (I guess 'warming' >> is 'warning'.) > You can tackle them in any order you wish. However, I suspect fixing the > xtensa backend may be easier. I don't have any good way to test xtensa, but > something like the attached patch for the xtensa port should be sufficient. > OK, thanks, I shall give a test for your patch, and I shall try to send patch for adding related warning for genrecog. I shall try to finish xtensa testsuite with xtensa qemu within next month (I am sure, xtensa qemu can work, and xtensa related maintainer is warm hearted: he even gives me suggestions for tilegx qemu). For tilegx qemu, I can run from _start to __libc_start_main correctly in linux-user mode, now. I shall try to finish tile qemu linux-user within next month (which may be enough for gcc testsuit, I guess) >> >> >> By the way, can this patch "initialize several arrays before use them >> in find_reloads()" will cause correctness issue? (I guess not, it >> will skip optimization, but not cause correctness issue, and continue >> compiling). > It will not cause any correctness issues, but it will cause > unwanted/unnecessary initialization of those arrays each time we enter that > function. > Oh, excuse me, my English is not quite well, I guess, you misunderstand my meaning, I want to say: - After this patch, it can continue compiling, but can we be sure that it always generates correct code for execution? - For this case, I can check the output assembly code for it (I shall try, next), but I can not be sure it will be true for any cases. For me, if we still do not initialize these arrays, we shall report error instead of warning when genrecog finds this issue. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/reload.c: Initialize several arrays before use them in find_reloads()
On 2/24/15 07:14, Jeff Law wrote: > On 02/23/15 16:09, Steven Bosscher wrote: >>> >>> >>> Which violate the rule for matching constraints. >> >> ...and should never have worked at all... > Yup. It's only been fairly recently that we started statically checking MD > files in any significant way -- we've still got a long way to go I'm sure. > OK, thanks. I shall try to finish within this month (although I am not quite sure whether I can finish on time). >> >> >>> I'm confident that if the xtensa's patterns were fixed to abide by the rules >>> for matching constraints the problem in reload would not occur. >> >> >> Chen, perhaps a warming can be implemented for this in genrecog? > That would certainly be a welcome addition! > OK, thanks. May I firstly finish it before fixing xtensa pattern? I guess it is more easier than fixing xtensa patten. (I guess 'warming' is 'warning'.) By the way, can this patch "initialize several arrays before use them in find_reloads()" will cause correctness issue? (I guess not, it will skip optimization, but not cause correctness issue, and continue compiling). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
[PATCH] gcc/reload.c: Initialize several arrays before use them in find_reloads()
It is for Bug65117, after this fix, ".i" file can be passed compiling.
- 'this_alternative_win' is not initialized before use it: for the
first looping 0, it initializes 'this_alternative_win[0]', but
'did_match' may use 'this_alternative_win[2]'.
- 'this_alternative' may be not initialized before using: it
initializes 'this_alternative[i]', but may use 'this_alternative[m]'
(m > i).
- After reading through the code, arrays 'this_alternative_match_win',
'this_alternative_offmemok', and 'this_alternative_earlyclobber' may
be not initialized either, so initialize them too.
This issue is found by cross compiling xtensa Linux kernel with the
latest gcc5. And after this patch, it can cross compile xtensa Linux
kernel with allmodconfig, successfully.
2015-02-22 Chen Gang
* reload.c (find_reloads): Initialize several arrays before use
them.
---
gcc/reload.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/gcc/reload.c b/gcc/reload.c
index 70b86a9..7e83c10 100644
--- a/gcc/reload.c
+++ b/gcc/reload.c
@@ -2666,10 +2666,10 @@ find_reloads (rtx_insn *insn, int replace, int
ind_levels, int live_known,
int no_input_reloads = 0, no_output_reloads = 0;
int n_alternatives;
reg_class_t this_alternative[MAX_RECOG_OPERANDS];
- char this_alternative_match_win[MAX_RECOG_OPERANDS];
- char this_alternative_win[MAX_RECOG_OPERANDS];
- char this_alternative_offmemok[MAX_RECOG_OPERANDS];
- char this_alternative_earlyclobber[MAX_RECOG_OPERANDS];
+ char this_alternative_match_win[MAX_RECOG_OPERANDS] = {0};
+ char this_alternative_win[MAX_RECOG_OPERANDS] = {0};
+ char this_alternative_offmemok[MAX_RECOG_OPERANDS] = {0};
+ char this_alternative_earlyclobber[MAX_RECOG_OPERANDS] = {0};
int this_alternative_matches[MAX_RECOG_OPERANDS];
reg_class_t goal_alternative[MAX_RECOG_OPERANDS];
int this_alternative_number;
@@ -2692,6 +2692,8 @@ find_reloads (rtx_insn *insn, int replace, int
ind_levels, int live_known,
machine_mode operand_mode[MAX_RECOG_OPERANDS];
int retval = 0;
+ for (i = 0; i < MAX_RECOG_OPERANDS; i++)
+this_alternative[i] = NO_REGS;
this_insn = insn;
n_reloads = 0;
n_replacements = 0;
--
1.9.3
[PATCH v3] libgcc: Use braces instead of macro's empty body to avoid xgcc warnings.
The related warning (cross compile tile with --disable-threads):
../../../../gcc-tile-new/libgcc/libgcov-interface.c: In function
'__gcov_fork':
../../../../gcc-tile-new/libgcc/libgcov-interface.c:182:53: warning: suggest
braces around empty body in an 'if' statement [-Wempty-body]
__GTHREAD_MUTEX_INIT_FUNCTION (&__gcov_flush_mx);
^
2015-01-31 Chen Gang
* gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Use empty do-
while loop as macro body.
---
libgcc/gthr-single.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libgcc/gthr-single.h b/libgcc/gthr-single.h
index f084fe2..01366f53 100644
--- a/libgcc/gthr-single.h
+++ b/libgcc/gthr-single.h
@@ -35,7 +35,7 @@ typedef int __gthread_recursive_mutex_t;
#define __GTHREAD_ONCE_INIT 0
#define __GTHREAD_MUTEX_INIT 0
-#define __GTHREAD_MUTEX_INIT_FUNCTION(mx)
+#define __GTHREAD_MUTEX_INIT_FUNCTION(mx) do {} while (0)
#define __GTHREAD_RECURSIVE_MUTEX_INIT 0
#define UNUSED __attribute__((unused))
--
1.9.3
Re: [PATCH v2] libgcc: Use braces instead of macro's empty body to avoid xgcc warnings.
On 1/31/15 19:49, Marek Polacek wrote: > On Sat, Jan 31, 2015 at 05:13:53PM +0530, Prathamesh Kulkarni wrote: >> On 31 January 2015 at 15:30, Chen Gang S wrote: >>> On 1/31/15 16:53, Andreas Schwab wrote: >>>> Chen Gang S writes: >>>> >>>>> * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Use braces >>>>> instead of macro's empty body to avoid xgcc warnings. >> The ChangeLog entry should be fixed to reflect it's an empty loop now. >> something like: >> * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Use empty do-while >> loop as macro body to avoid xgcc warnings. > > And since CL entries should describe what, not why, the "to avoid..." > part is redundant. > OK, thanks. I should send patch v3 within today. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH v2] libgcc: Use braces instead of macro's empty body to avoid xgcc warnings.
On 1/31/15 16:53, Andreas Schwab wrote: > Chen Gang S writes: > >> * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Use braces >> instead of macro's empty body to avoid xgcc warnings. > > It's actually an empty loop now. > Excuse me, I am not quite clear gcc version merging working flow. Now, for gcc master branch (20150131), __GTHREAD_MUTEX_INIT_FUNCTION is still as empty. Could you provide more details about it? Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
[PATCH v2] libgcc: Use braces instead of macro's empty body to avoid xgcc warnings.
The related warning (cross compile tile with --disable-threads):
../../../../gcc-tile-new/libgcc/libgcov-interface.c: In function
'__gcov_fork':
../../../../gcc-tile-new/libgcc/libgcov-interface.c:182:53: warning: suggest
braces around empty body in an 'if' statement [-Wempty-body]
__GTHREAD_MUTEX_INIT_FUNCTION (&__gcov_flush_mx);
^
2015-01-31 Chen Gang
* gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Use braces
instead of macro's empty body to avoid xgcc warnings.
---
libgcc/gthr-single.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libgcc/gthr-single.h b/libgcc/gthr-single.h
index f084fe2..01366f53 100644
--- a/libgcc/gthr-single.h
+++ b/libgcc/gthr-single.h
@@ -35,7 +35,7 @@ typedef int __gthread_recursive_mutex_t;
#define __GTHREAD_ONCE_INIT 0
#define __GTHREAD_MUTEX_INIT 0
-#define __GTHREAD_MUTEX_INIT_FUNCTION(mx)
+#define __GTHREAD_MUTEX_INIT_FUNCTION(mx) do {} while (0)
#define __GTHREAD_RECURSIVE_MUTEX_INIT 0
#define UNUSED __attribute__((unused))
--
1.9.3
Re: [PATCH] libgcc: Use braces instead of macro's empty body to avoid xgcc warnings.
On 1/31/15 06:57, Jakub Jelinek wrote:
> On Sat, Jan 31, 2015 at 06:37:30AM +0800, Chen Gang S wrote:
>> The related warning (cross compile tile with --disable-threads):
>>
>> ../../../../gcc-tile-new/libgcc/libgcov-interface.c: In function
>> '__gcov_fork':
>> ../../../../gcc-tile-new/libgcc/libgcov-interface.c:182:53: warning:
>> suggest braces around empty body in an 'if' statement [-Wempty-body]
>>__GTHREAD_MUTEX_INIT_FUNCTION (&__gcov_flush_mx);
>>^
>
> Then you should use do {} while (0) instead of {} . Also, braces not breaces.
>
OK, thanks, I shall send patch v2 within today (but maybe at night).
>> 2015-01-31 Chen Gang
>>
>> * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Use breaces
>> instead of macro's empty body to avoid xgcc warnings.
>> ---
>> libgcc/gthr-single.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libgcc/gthr-single.h b/libgcc/gthr-single.h
>> index f084fe2..586fec5 100644
>> --- a/libgcc/gthr-single.h
>> +++ b/libgcc/gthr-single.h
>> @@ -35,7 +35,7 @@ typedef int __gthread_recursive_mutex_t;
>>
>> #define __GTHREAD_ONCE_INIT 0
>> #define __GTHREAD_MUTEX_INIT 0
>> -#define __GTHREAD_MUTEX_INIT_FUNCTION(mx)
>> +#define __GTHREAD_MUTEX_INIT_FUNCTION(mx) {}
>> #define __GTHREAD_RECURSIVE_MUTEX_INIT 0
>>
>> #define UNUSED __attribute__((unused))
>> --
>> 1.9.3
>
> Jakub
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
[PATCH] libgcc: Use braces instead of macro's empty body to avoid xgcc warnings.
The related warning (cross compile tile with --disable-threads):
../../../../gcc-tile-new/libgcc/libgcov-interface.c: In function
'__gcov_fork':
../../../../gcc-tile-new/libgcc/libgcov-interface.c:182:53: warning: suggest
braces around empty body in an 'if' statement [-Wempty-body]
__GTHREAD_MUTEX_INIT_FUNCTION (&__gcov_flush_mx);
^
2015-01-31 Chen Gang
* gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Use breaces
instead of macro's empty body to avoid xgcc warnings.
---
libgcc/gthr-single.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libgcc/gthr-single.h b/libgcc/gthr-single.h
index f084fe2..586fec5 100644
--- a/libgcc/gthr-single.h
+++ b/libgcc/gthr-single.h
@@ -35,7 +35,7 @@ typedef int __gthread_recursive_mutex_t;
#define __GTHREAD_ONCE_INIT 0
#define __GTHREAD_MUTEX_INIT 0
-#define __GTHREAD_MUTEX_INIT_FUNCTION(mx)
+#define __GTHREAD_MUTEX_INIT_FUNCTION(mx) {}
#define __GTHREAD_RECURSIVE_MUTEX_INIT 0
#define UNUSED __attribute__((unused))
--
1.9.3
Re: [PATCH] libiberty/argv.c: Use freeargv() instead of free() to avoid memory leak.
On 1/28/15 20:02, Andrew Burgess wrote: > * Chen Gang S [2015-01-28 19:34:38 +0800]: > >> libiberty/argv.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libiberty/argv.c b/libiberty/argv.c >> index f2727e8..9fdd55b 100644 >> --- a/libiberty/argv.c >> +++ b/libiberty/argv.c >> @@ -454,7 +454,7 @@ expandargv (int *argcp, char ***argvp) >>/* Free up memory allocated to process the response file. We do >> not use freeargv because the individual options in FILE_ARGV >> are now in the main ARGV. */ >> - free (file_argv); >> + freeargv (file_argv); > > I'm not commenting on the correctness of the patch, however, you will > need to update the comment just above this change. > > Personally, I'd also hope that the commit message explains why the > comment is no longer correct, rather than just stating that the change > has been made. > Oh, sorry! After read the comments carefully, for me, it doesn't need additional explanation. Sorry for my carelessness, also excuse me for my poor English. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] libiberty/argv.c: Use freeargv() instead of free() to avoid memory leak.
On 1/28/15 20:02, Andrew Burgess wrote: > * Chen Gang S [2015-01-28 19:34:38 +0800]: > >> libiberty/argv.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libiberty/argv.c b/libiberty/argv.c >> index f2727e8..9fdd55b 100644 >> --- a/libiberty/argv.c >> +++ b/libiberty/argv.c >> @@ -454,7 +454,7 @@ expandargv (int *argcp, char ***argvp) >>/* Free up memory allocated to process the response file. We do >> not use freeargv because the individual options in FILE_ARGV >> are now in the main ARGV. */ >> - free (file_argv); >> + freeargv (file_argv); > > I'm not commenting on the correctness of the patch, however, you will > need to update the comment just above this change. > > Personally, I'd also hope that the commit message explains why the > comment is no longer correct, rather than just stating that the change > has been made. > Oh, really. Excuse me, originally I did not notice about it. I shall send patch v2 for it (just like what DJ Delorie said in another reply). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
[PATCH] libiberty/argv.c: Use freeargv() instead of free() to avoid memory leak.
Need free each array elements, or may cause memory leak. 2015-01-28 Chen Gang * argv.c (expandargv): Use freeargv() instead of free() to avoid memory leak. --- libiberty/argv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libiberty/argv.c b/libiberty/argv.c index f2727e8..9fdd55b 100644 --- a/libiberty/argv.c +++ b/libiberty/argv.c @@ -454,7 +454,7 @@ expandargv (int *argcp, char ***argvp) /* Free up memory allocated to process the response file. We do not use freeargv because the individual options in FILE_ARGV are now in the main ARGV. */ - free (file_argv); + freeargv (file_argv); free (buffer); /* Rescan all of the arguments just read to support response files that include other response files. */ -- 1.9.3 (Apple Git-50)
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
On 1/27/15 05:56, Jeff Law wrote: > On 01/22/15 13:50, Chen Gang S wrote: >> On 01/23/2015 03:53 AM, Jeff Law wrote: >>> On 01/22/15 12:42, Chen Gang S wrote: >>>> >>>> >>>> Thank you very much for your help applying the 3 patches. :-) >>> No problem. >>> >>>> >>>> After finish the assignment working flow, I guess, I may have the write >>>> access, then can finish these kinds of trivial patches by myself (will >>>> not always bother other members any more). >>> Exactly. They'll still need review/approval, but once approved, you'll be >>> able to commit them yourself. >>> >> >> OK, thanks. I guess that is "Write After Approval" maintainer role. >> >> And I want to consult: for passing assignment working flow, must I make >> a patch which contents much code lines? (At present, my patches are all >> trivial patch). >> >> If really need a patch which contents enough code lines, please let me >> know, and I shall try. > You can get write-after-approval access regardless of the size and complexity > of your patches. > OK, thanks. :-) I sent the my related information email, and wait reply. After get reply (get assignment pdf template), I shall post my assignment paper, it may need almost a month (post from China to USA may need 1 month). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
On 1/26/15 03:50, Gerald Pfeifer wrote: > On Friday 2015-01-23 04:50, Chen Gang S wrote: >> And I want to consult: for passing assignment working flow, must I >> make a patch which contents much code lines? (At present, my patches >> are all trivial patch). > > No, the assignment process does not have any requirements on > past or current patches. You even can complete the assignment > process without a single patch at that point. > > So, do not worry. > > And we are happy to see improvements and fixes of all kinds, > so please continue. > Thank you very much for your encourage. :-) -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
On 01/23/2015 03:53 AM, Jeff Law wrote: > On 01/22/15 12:42, Chen Gang S wrote: >> >> >> Thank you very much for your help applying the 3 patches. :-) > No problem. > >> >> After finish the assignment working flow, I guess, I may have the write >> access, then can finish these kinds of trivial patches by myself (will >> not always bother other members any more). > Exactly. They'll still need review/approval, but once approved, you'll be > able to commit them yourself. > OK, thanks. I guess that is "Write After Approval" maintainer role. And I want to consult: for passing assignment working flow, must I make a patch which contents much code lines? (At present, my patches are all trivial patch). If really need a patch which contents enough code lines, please let me know, and I shall try. Thanks. -- Open, share, and attitude like air, water, and life which God blessed.
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
On 01/23/2015 12:41 AM, Jeff Law wrote: > On 01/22/15 04:55, Chen Gang S wrote: >> On 01/22/2015 02:46 AM, Mike Stump wrote: >>> On Jan 21, 2015, at 1:54 AM, Chen Gang S wrote: >>>> On 1/6/15 04:07, Jeff Law wrote: >>>>> >>>>>> * toplev.c (init_local_tick): Process the failure when read >>>>>> fails for random_seed. >>>>> This is fine for the trunk. Please install. >>> >>>> I am not familiar with the related working flow, who should >>>> install this patch? >>> >>> Anyone with write privileges can do this. You can request installation of >>> the patch with the magic phrase: >>> >>>I don’t have write access yet nor is my paperwork compete, can someone >>> install this for me? >>> >>> This will prompt a volunteer to step forward and help get it installed. >>> When they are done, they will say, installed, close any related bug report >>> and if they are nice, tell you the commit number for the work. You can >>> then update your tree, build and test and ensure the work is as you wanted >>> it. Once it hits the trunk, you’re done. If the patch is important enough >>> for a back-port, you can ask people to consider a back port, once the work >>> has been proven on the trunk for a week or two. >>> >> >> OK, thanks, for me, this is only a trivial patch (not urgent). So I can >> understand why it is still pending. >> >> And if possible, could you help install this patch when you have time? > Installed on your behalf. > Thank you very much for your help applying the 3 patches. :-) After finish the assignment working flow, I guess, I may have the write access, then can finish these kinds of trivial patches by myself (will not always bother other members any more). Thanks. -- Open, share, and attitude like air, water, and life which God blessed.
Re: [PATCH] libgcc: unwind-dw2-fde.h: Use "(const fde *)" instead of "(char *)" to avoid qualifier warning
On 10/08/2014 10:52 PM, Richard Henderson wrote: > On 10/08/2014 03:47 AM, Chen Gang wrote: >> It passes "make -k check" under Darwin x86_64. >> >> 2014-10-07 Chen Gang >> >> * unwind-dw2-fde.h (last_fde): Use "(const fde *)" instead of >> "(char *)" to avoid qualifier warning by 'xgcc' compiling. > > Ok. > Hello Maintainers: At present, I don’t have write access yet nor is my paperwork compete, could someone help install it for me? Thanks. -- Chen Gang Open share and attitude like air water and life which God blessed -- Open, share, and attitude like air, water, and life which God blessed.
Re: [PATCH v3] gcc/ubsan.c: Use 'pretty_print' for 'pretty_name' to avoid memory overflow
On 11/24/2014 04:24 PM, Jakub Jelinek wrote: > On Mon, Nov 24, 2014 at 04:28:10PM +0800, Chen Gang wrote: >> On 11/24/14 15:41, Jakub Jelinek wrote: >>> On Sun, Nov 23, 2014 at 09:13:27AM +0800, Chen Gang wrote: >> >> [...] >> >>>> +else >>>> + pp_wide_int(&pretty_name, >>>> + wi::add (wi::to_widest (TYPE_MAX_VALUE (dom)), 1), >>>> + TYPE_SIGN (TREE_TYPE (dom))); >>> >>> Space still missing before ( (and reindenting the following 2 lines). >>> >> >> Oh, thanks, if necessary to send patch v4, please let me know. > > No, just fix it up before checking in. > Hello Maintainers: At present, I don’t have write access yet nor is my paperwork compete, could someone help install it for me? Thanks. -- Chen Gang Open share and attitude like air water and life which God blessed -- Open, share, and attitude like air, water, and life which God blessed.
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
On 01/22/2015 02:46 AM, Mike Stump wrote: > On Jan 21, 2015, at 1:54 AM, Chen Gang S wrote: >> On 1/6/15 04:07, Jeff Law wrote: >>> >>>>* toplev.c (init_local_tick): Process the failure when read >>>>fails for random_seed. >>> This is fine for the trunk. Please install. > >> I am not familiar with the related working flow, who should >> install this patch? > > Anyone with write privileges can do this. You can request installation of > the patch with the magic phrase: > > I don’t have write access yet nor is my paperwork compete, can someone > install this for me? > > This will prompt a volunteer to step forward and help get it installed. When > they are done, they will say, installed, close any related bug report and if > they are nice, tell you the commit number for the work. You can then update > your tree, build and test and ensure the work is as you wanted it. Once it > hits the trunk, you’re done. If the patch is important enough for a > back-port, you can ask people to consider a back port, once the work has been > proven on the trunk for a week or two. > OK, thanks, for me, this is only a trivial patch (not urgent). So I can understand why it is still pending. And if possible, could you help install this patch when you have time? :-) Thanks. -- Open, share, and attitude like air, water, and life which God blessed.
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
On 1/6/15 04:07, Jeff Law wrote: > On 12/28/14 21:04, Chen Gang S wrote: >> When failure occurs, random_seed may not be 0, so need reset it to 0 >> manually, or will let the next call init_random_seed() incorrect. The >> related warning: >> >>g++ -c -DTARGET_NAME=\"parisc-gchen-linux\" -g -O2 -DIN_GCC >> -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti >> -fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual >> -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long >> -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H >> -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include >> -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libdecnumber >> -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber >> -I../../gcc/gcc/../libbacktrace -o toplev.o -MT toplev.o -MMD -MP -MF >> ./.deps/toplev.TPo ../../gcc/gcc/toplev.c >>../../gcc/gcc/toplev.c: In function 'void init_local_tick()': >>../../gcc/gcc/toplev.c:276:56: warning: ignoring return value of 'ssize_t >> read(int, void*, size_t)', declared with attribute warn_unused_result >> [-Wunused-result] >> >> It passes testsuite under x86_64-apple-darwin14.0.0 >> >> /gcc >> 2014-12-27 Chen Gang >> >> * toplev.c (init_local_tick): Process the failure when read >> fails for random_seed. > This is fine for the trunk. Please install. > Excuse me, I am not familiar with the related working flow, who should install this patch? At present, I can not find the related patch in gcc master branch. If I should install the patch by myself, sorry, at present, I have no write priority. I am performing the assignment working flow for gcc, I guess, after finish the working flow, I may have write priority. If what I said above is incorrect, or the time is too long to bear (I guess, may need 1 month to finish the assignment working flow), welcome any members to help install the patch. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
On 1/20/15 10:11, Joseph Myers wrote: > On Mon, 19 Jan 2015, Chen Gang S wrote: > >> On 12/31/2014 06:26 AM, Joseph Myers wrote: >>> On Mon, 29 Dec 2014, Chen Gang S wrote: >>> >>>> And in honest, this year what I have done is really not quite well, next >>>> year I should be improved: should scanning Bugzilla and try to fix the >>>> existing issues (just like another members' suggestions to me). >>> >>> Note that for any substantial patches you'll need to complete the >>> copyright assignment paperwork (I don't see you listed in copyright.list >>> at present). >>> >> >> Excuse me, I am not quite familiar with the related working flow, at >> present, I finished assignment paperwork for binutils and gdb, and I >> am one of write after approval member for binutils and gdb. >> >> Do you mean I need follow the same working flow for gcc, just as for the >> binutils and gdb? (or only post my assignment is OK?). > > You need to complete the same assignment form. Start with > > http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future > > but this time name GCC as the package. > OK, thanks, I will follow the related working flow. And I shall try to post my assignment paper within this month. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
On 12/31/2014 06:26 AM, Joseph Myers wrote: > On Mon, 29 Dec 2014, Chen Gang S wrote: > >> And in honest, this year what I have done is really not quite well, next >> year I should be improved: should scanning Bugzilla and try to fix the >> existing issues (just like another members' suggestions to me). > > Note that for any substantial patches you'll need to complete the > copyright assignment paperwork (I don't see you listed in copyright.list > at present). > Excuse me, I am not quite familiar with the related working flow, at present, I finished assignment paperwork for binutils and gdb, and I am one of write after approval member for binutils and gdb. Do you mean I need follow the same working flow for gcc, just as for the binutils and gdb? (or only post my assignment is OK?). If possible, please provide more related details for it. I shall follow. Thanks. -- Open, share, and attitude like air, water, and life which God blessed.
Re: [PATCH] config/h8300/h8300.c: Regress part of the original commit for fixing issue
On 1/13/15 01:32, Jeff Law wrote: > On 01/12/15 10:01, Jeff Law wrote: >> This indicates a violation of the type safety invariants we're adding to >> GCC. Simply changing the code to use rtx rather than rtx_insn is >> probably a step in the wrong direction. >> >> Part of the problem here is that RTX_FRAME_RELATED_P is valid on both >> rtx_insn and rtx objects. That's something we'll have to fix as the >> type safety work moves forward, assuming we continue towards the goal of >> totally separating rtx and rtx_insn objects. >> >> Returning to the code in h8300.c, we have "F" which assumes its argument >> is an rtx_insn. We should never be calling "F" will anything other than >> an rtx_insn argument. The calls from "Fpa" are the only violators of >> that invariant. >> Oh, what you said above sound reasonable to me. "F" is also used by others which pass rtx_insn to "F". >> Given that the vectors inside the PARALLEL will always be rtx objects >> and that we always want to set RTX_FRAME_RELATED on those objects, it >> seems that we could just replace the call to "F" in "Fpa" with >> >> RTX_FRAME_RELATED_P (XVECEXP (par, 0, i)) = 1; >> >> >> That simplifies the code and removes a bogus as_a cast. >> Yeah, for me, it is a good idea. >> Can you try that and report back to me? > Nevermind. I went ahead and made this change and verified that libgcc, > libquadmath and newlib would build for the h8300-elf configuration. > Thank you for your work. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
[PATCH] config/h8300/h8300.c: Regress part of the original commit for fixing issue
The related commit "1a1ed14 config/h8300: Use rtx_insn" gives an extra
check for rtx, which will cause building libgcc break, after regress it,
it can still generate the correct assemble code.
The related information is below:
[root@localhost libgcc]# cat libgcc2.i
typedef int DItype __attribute__ ((mode (DI)));
DItype __muldi3 (DItype u, DItype v)
{
return u + v;
}
[root@localhost libgcc]# /upstream/build-gcc-h8300/gcc/cc1 -ms -O2 libgcc2.i
__muldi3
Analyzing compilation unit
Performing interprocedural optimizations
<*free_lang_data>
Assembling functions:
__muldi3
libgcc2.i: In function '__muldi3':
libgcc2.i:5:1: internal compiler error: in as_a, at is-a.h:192
}
^
0xce2aef as_a
../../gcc/gcc/is-a.h:192
0xce2aef Fpa
../../gcc/gcc/config/h8300/h8300.c:530
0xce2aef h8300_push_pop
../../gcc/gcc/config/h8300/h8300.c:724
0xce33e3 h8300_push_pop
../../gcc/gcc/config/h8300/h8300.c:869
0xce33e3 h8300_expand_prologue()
../../gcc/gcc/config/h8300/h8300.c:890
0xcee69a gen_prologue()
../../gcc/gcc/config/h8300/h8300.md:2651
0x7c0b19 thread_prologue_and_epilogue_insns()
../../gcc/gcc/function.c:5923
0x7c1032 rest_of_handle_thread_prologue_and_epilogue
../../gcc/gcc/function.c:6493
0x7c1032 execute
../../gcc/gcc/function.c:6531
After this patch, it can generate the correct assembly code:
[root@localhost libgcc]# h8300-gchen-elf-gcc -ms -O2 -S libgcc2.i
[root@localhost libgcc]# cat ./libgcc2.s
.file "libgcc2.i"
.h8300s
.section .text
.align 1
.global ___muldi3
___muldi3:
mov.l er6,@-er7
mov.l er7,er6
stm.l er4-er5,@-er7
sub.l #12,er7
mov.l er0,er2
mov.l er1,er3
mov.l @(8,er6),er0
mov.l er0,@(-20,er6)
mov.l @(12,er6),er0
mov.l er0,@(-16,er6)
mov.l er0,er5
add.l er1,er5
sub.l er1,er1
add.b #1,r1l
cmp.l er3,er5
blo .L2
sub.l er1,er1
.L2:
mov.l @(-20,er6),er4
add.l er2,er4
mov.l er1,er0
add.l er4,er0
mov.l er5,er1
add.l #12,er7
ldm.l @er7+,er4-er5
mov.l @er7+,er6
rts
.size ___muldi3, .-___muldi3
.ident "GCC: (GNU) 5.0.0 20150109 (experimental)"
.end
For mode(DI), it generates 64-bit integer, so it uses er0 and er1 as
parameter 1, and stack (8,er6) and (12,er6) for parameter 2, return
value is er0 and er1. And the internal algorithim is also correct.
2015-01-11 Chen Gang
* config/h8300/h8300.c (F): Use rtx instead of rtx_insn *.
(Fpa): Remove additional check by rtx_insn *.
---
gcc/config/h8300/h8300.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gcc/config/h8300/h8300.c b/gcc/config/h8300/h8300.c
index fe85df5..994c38f 100644
--- a/gcc/config/h8300/h8300.c
+++ b/gcc/config/h8300/h8300.c
@@ -506,8 +506,8 @@ byte_reg (rtx x, int b)
&& !crtl->is_leaf)))
/* We use this to wrap all emitted insns in the prologue. */
-static rtx_insn *
-F (rtx_insn *x, bool set_it)
+static rtx
+F (rtx x, bool set_it)
{
if (set_it)
RTX_FRAME_RELATED_P (x) = 1;
@@ -527,7 +527,7 @@ Fpa (rtx par)
int i;
for (i = 0; i < len; i++)
-F (as_a (XVECEXP (par, 0, i)), true);
+F (XVECEXP (par, 0, i), true);
return par;
}
--
1.9.3
[PATCH] ipa-icf.c: Fix issues generated by original latest commit
The related commit is "275e275 IPA ICF: target and optimization flags
comparison.". For sem_function::equals_private(), fix the typo issue,
and for target_opts_for_fn(), fix access NULL issue.
For cross compiling h8300, it will cause the issue below:
[root@localhost h8300]# cat fp-bit.i
__inline__ static int a (int x)
{
return __builtin_expect (x == 0, 0);
}
__inline__ static int b (int x)
{
return __builtin_expect (x == 1, 0);
}
__attribute__ ((__always_inline__)) int c (int x, int y)
{
if (a (x))
return x;
if (b (x))
return x;
return y;
}
[root@localhost h8300]# /upstream/build-gcc-h8300/gcc/cc1 -O2 fp-bit.i -o
test.s
a b c
Analyzing compilation unit
fp-bit.i:11:41: warning: always_inline function might not be inlinable
[-Wattributes]
__attribute__ ((__always_inline__)) int c (int x, int y)
^
Performing interprocedural optimizations
<*free_lang_data>
fp-bit.i:18:1: internal compiler error: Segmentation
fault
}
^
0xa11f0e crash_signal
../../gcc/gcc/toplev.c:372
0xda33e7 tree_check
../../gcc/gcc/tree.h:2769
0xda33e7 target_opts_for_fn
../../gcc/gcc/tree.h:4643
0xda33e7 ipa_icf::sem_function::equals_private(ipa_icf::sem_item*,
hash_map&)
../../gcc/gcc/ipa-icf.c:438
0xda4023 ipa_icf::sem_function::equals(ipa_icf::sem_item*,
hash_map&)
../../gcc/gcc/ipa-icf.c:393
0xda6472 ipa_icf::sem_item_optimizer::subdivide_classes_by_equality(bool)
../../gcc/gcc/ipa-icf.c:1900
0xdaad3c ipa_icf::sem_item_optimizer::execute()
../../gcc/gcc/ipa-icf.c:1719
0xdab961 ipa_icf_driver
../../gcc/gcc/ipa-icf.c:2448
0xdab961 ipa_icf::pass_ipa_icf::execute(function*)
../../gcc/gcc/ipa-icf.c:2496
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
This issue can be found for cross compiling gcc "make all-target-libgcc"
under h8300, after fix this issue, it can continue to cross compiling to
meet the next building issue for h8300.
2015-01-10 Chen Gang
* ipa-icf.c (sem_function::equals_private): Use '&&' instead of
'||' to fix typo issue.
* gcc/tree.h (target_opts_for_fn): Check NULL_TREE since it can
accept and return NULL.
---
gcc/ipa-icf.c | 2 +-
gcc/tree.h| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 1b76a1d..4ccaf8c 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -438,7 +438,7 @@ sem_function::equals_private (sem_item *item,
cl_target_option *tar1 = target_opts_for_fn (decl);
cl_target_option *tar2 = target_opts_for_fn (m_compared_func->decl);
- if (tar1 != NULL || tar2 != NULL)
+ if (tar1 != NULL && tar2 != NULL)
{
if (!cl_target_option_eq (tar1, tar2))
{
diff --git a/gcc/tree.h b/gcc/tree.h
index fc8c8fe..ac27268 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4640,7 +4640,7 @@ target_opts_for_fn (const_tree fndecl)
tree fn_opts = DECL_FUNCTION_SPECIFIC_TARGET (fndecl);
if (fn_opts == NULL_TREE)
fn_opts = target_option_default_node;
- return TREE_TARGET_OPTION (fn_opts);
+ return fn_opts == NULL_TREE ? NULL : TREE_TARGET_OPTION (fn_opts);
}
/* opt flag for function FNDECL, e.g. opts_for_fn (fndecl, optimize) is
--
1.9.3
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
On 1/6/15 04:07, Jeff Law wrote: > On 12/28/14 21:04, Chen Gang S wrote: >> When failure occurs, random_seed may not be 0, so need reset it to 0 >> manually, or will let the next call init_random_seed() incorrect. The >> related warning: >> >>g++ -c -DTARGET_NAME=\"parisc-gchen-linux\" -g -O2 -DIN_GCC >> -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti >> -fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual >> -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long >> -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H >> -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include >> -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libdecnumber >> -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber >> -I../../gcc/gcc/../libbacktrace -o toplev.o -MT toplev.o -MMD -MP -MF >> ./.deps/toplev.TPo ../../gcc/gcc/toplev.c >>../../gcc/gcc/toplev.c: In function 'void init_local_tick()': >>../../gcc/gcc/toplev.c:276:56: warning: ignoring return value of 'ssize_t >> read(int, void*, size_t)', declared with attribute warn_unused_result >> [-Wunused-result] >> >> It passes testsuite under x86_64-apple-darwin14.0.0 >> >> /gcc >> 2014-12-27 Chen Gang >> >> * toplev.c (init_local_tick): Process the failure when read >> fails for random_seed. > This is fine for the trunk. Please install. > Thank you for your work. This year, I shall try the Bugzilla for the existing issues, firstly, I should try to continue to analyze the old issues for h8300 which I reported in 2013 (1.5 years ago). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
Hello Maintainers: It is finished with qemu today, it spends 13 days, the result is below, please help check. With qemu: === gcc Summary === # of expected passes83439 # of unexpected failures175 # of unexpected successes 1 # of expected failures 100 # of unresolved testcases 349 # of unsupported tests 1954 /upstream/build-gcc-microblaze/gcc/xgcc version 5.0.0 20141220 (experimental) (GCC) No qemu (originally): === gcc Summary === # of expected passes65987 # of unexpected failures82 # of unexpected successes 1 # of expected failures 97 # of unresolved testcases 16378 # of unsupported tests 1810 If the result is OK, I shall start tile environments construction ( although, I guess, there is no qemu or sim for tile). And also excuse me, during these days, gmail is broken in China, I have to use my another mail address to send/receive emails. Thanks. On 12/21/14 05:45, Chen Gang wrote: > On 12/21/2014 12:31 AM, Michael Eager wrote: >> On 12/20/14 02:09, Chen Gang wrote: >>> By the way, if this thread really has negative effect with other members, >>> please warn me, I should not notify it to mailing list again, and try my >>> best to finish it within myself. >> >> I appreciate your enthusiasm and perseverance in pursuing this bug. >> >> If the problem you are working on has changed from the mb-gcc issue, >> change the subject. Otherwise, keep up the good work. >> > > Thank you for your encouragement, and I should continue. At present, I > guess my own main issues are: > > - Have no enough time resources on open source: > > sometimes need work overtime. > > need 4 hours per work day on subway between home and work office > (come 2 hours, go 2 hours, so total is 4 hours). > > need spend time for my child: check his homework, play with him. > (especially in weekend). > > - Really not familiar with gcc: > > Sometimes can find real world issues, but can not fix them in time. > > Sometimes can find coding issues, but do not know whether it can > cause real world issues or not (may also waste other members time > resources, but get no positive result). > > - Not familiar with related environments for each architectures. > > So next, I should change myself for solving the issues above, firstly: > > - I shall try to spend 1-1.5 hours for reading gcc related documents ( >e.g. "gcc info") in work day when I on subway (another time on subway >is for listening Holy Bible, reading news, or sleeping for a while). > > - I shall mainly forcus on finding real world issues and try to fix in >time. And stop finding coding issues (which may get negative effect >with others -- at least may waste other members time resources). > > - Still contiue for constructing all related enviroments (it is always >necessary) for architectures. > > Welcome any ideas, suggestions or completions by any members. > > Thanks. > -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
On 12/31/14 06:26, Joseph Myers wrote: > On Mon, 29 Dec 2014, Chen Gang S wrote: > >> And in honest, this year what I have done is really not quite well, next >> year I should be improved: should scanning Bugzilla and try to fix the >> existing issues (just like another members' suggestions to me). > > Note that for any substantial patches you'll need to complete the > copyright assignment paperwork (I don't see you listed in copyright.list > at present). > OK, thanks. At present, I am just process about binutils and gdb for it. I have printed the paper from FSF, sign my name and date, and posted it to FSF in Dec-28-2014, (I guess, it may reach to FSF within 1 month). After finish the working flow about binutils and gdb, I shall start the related working flow for gcc. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] toplev.c: Process the failure when read fails for random_seed
Excuse me, I have to send mail use my other mail address.
During these days, gmail is broken in China, except Safari from Mac Book,
none of my email client can connect to gmail (even Safari from iPad, can
not connect gmail).
And in honest, this year what I have done is really not quite well, next
year I should be improved: should scanning Bugzilla and try to fix the
existing issues (just like another members' suggestions to me).
Thanks.
On 12/29/14 12:04, Chen Gang S wrote:
> When failure occurs, random_seed may not be 0, so need reset it to 0
> manually, or will let the next call init_random_seed() incorrect. The
> related warning:
>
> g++ -c -DTARGET_NAME=\"parisc-gchen-linux\" -g -O2 -DIN_GCC
> -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti
> -fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual
> -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long
> -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -I.
> -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include
> -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libdecnumber
> -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber
> -I../../gcc/gcc/../libbacktrace -o toplev.o -MT toplev.o -MMD -MP -MF
> ./.deps/toplev.TPo ../../gcc/gcc/toplev.c
> ../../gcc/gcc/toplev.c: In function 'void init_local_tick()':
> ../../gcc/gcc/toplev.c:276:56: warning: ignoring return value of 'ssize_t
> read(int, void*, size_t)', declared with attribute warn_unused_result
> [-Wunused-result]
>
> It passes testsuite under x86_64-apple-darwin14.0.0
>
> /gcc
> 2014-12-27 Chen Gang
>
> * toplev.c (init_local_tick): Process the failure when read
> fails for random_seed.
> ---
> gcc/toplev.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index 7e06247..e5262be 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -273,7 +273,9 @@ init_local_tick (void)
>int fd = open ("/dev/urandom", O_RDONLY);
>if (fd >= 0)
> {
> - read (fd, &random_seed, sizeof (random_seed));
> + if (read (fd, &random_seed, sizeof (random_seed))
> + != sizeof (random_seed))
> +random_seed = 0;
>close (fd);
> }
>
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
[PATCH] toplev.c: Process the failure when read fails for random_seed
When failure occurs, random_seed may not be 0, so need reset it to 0
manually, or will let the next call init_random_seed() incorrect. The
related warning:
g++ -c -DTARGET_NAME=\"parisc-gchen-linux\" -g -O2 -DIN_GCC
-DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti
-fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual
-Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long
-Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -I.
-I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include
-I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libdecnumber
-I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber
-I../../gcc/gcc/../libbacktrace -o toplev.o -MT toplev.o -MMD -MP -MF
./.deps/toplev.TPo ../../gcc/gcc/toplev.c
../../gcc/gcc/toplev.c: In function 'void init_local_tick()':
../../gcc/gcc/toplev.c:276:56: warning: ignoring return value of 'ssize_t
read(int, void*, size_t)', declared with attribute warn_unused_result
[-Wunused-result]
It passes testsuite under x86_64-apple-darwin14.0.0
/gcc
2014-12-27 Chen Gang
* toplev.c (init_local_tick): Process the failure when read
fails for random_seed.
---
gcc/toplev.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 7e06247..e5262be 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -273,7 +273,9 @@ init_local_tick (void)
int fd = open ("/dev/urandom", O_RDONLY);
if (fd >= 0)
{
- read (fd, &random_seed, sizeof (random_seed));
+ if (read (fd, &random_seed, sizeof (random_seed))
+ != sizeof (random_seed))
+random_seed = 0;
close (fd);
}
--
1.9.3 (Apple Git-50)
From 05faeede37f6e383f69a1398d355ea8ce3687583 Mon Sep 17 00:00:00 2001
From: Chen Gang
Date: Sat, 27 Dec 2014 15:55:57 +0800
Subject: [PATCH] toplev.c: Process the failure when read fails for random_seed
When failure occurs, random_seed may not be 0, so need reset it to 0
manually, or will let the next call init_random_seed() incorrect. The
related warning:
g++ -c -DTARGET_NAME=\"parisc-gchen-linux\" -g -O2 -DIN_GCC
-DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti
-fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual
-Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long
-Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -I.
-I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include
-I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libdecnumber
-I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber
-I../../gcc/gcc/../libbacktrace -o toplev.o -MT toplev.o -MMD -MP -MF
./.deps/toplev.TPo ../../gcc/gcc/toplev.c
../../gcc/gcc/toplev.c: In function 'void init_local_tick()':
../../gcc/gcc/toplev.c:276:56: warning: ignoring return value of 'ssize_t
read(int, void*, size_t)', declared with attribute warn_unused_result
[-Wunused-result]
It passes testsuite under x86_64-apple-darwin14.0.0
/gcc
2014-12-27 Chen Gang
* toplev.c (init_local_tick): Process the failure when read
fails for random_seed.
---
gcc/toplev.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 7e06247..e5262be 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -273,7 +273,9 @@ init_local_tick (void)
int fd = open ("/dev/urandom", O_RDONLY);
if (fd >= 0)
{
- read (fd, &random_seed, sizeof (random_seed));
+ if (read (fd, &random_seed, sizeof (random_seed))
+ != sizeof (random_seed))
+random_seed = 0;
close (fd);
}
--
1.9.3 (Apple Git-50)
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
On 12/21/2014 12:31 AM, Michael Eager wrote: > On 12/20/14 02:09, Chen Gang wrote: >> By the way, if this thread really has negative effect with other members, >> please warn me, I should not notify it to mailing list again, and try my >> best to finish it within myself. > > I appreciate your enthusiasm and perseverance in pursuing this bug. > > If the problem you are working on has changed from the mb-gcc issue, > change the subject. Otherwise, keep up the good work. > Thank you for your encouragement, and I should continue. At present, I guess my own main issues are: - Have no enough time resources on open source: sometimes need work overtime. need 4 hours per work day on subway between home and work office (come 2 hours, go 2 hours, so total is 4 hours). need spend time for my child: check his homework, play with him. (especially in weekend). - Really not familiar with gcc: Sometimes can find real world issues, but can not fix them in time. Sometimes can find coding issues, but do not know whether it can cause real world issues or not (may also waste other members time resources, but get no positive result). - Not familiar with related environments for each architectures. So next, I should change myself for solving the issues above, firstly: - I shall try to spend 1-1.5 hours for reading gcc related documents ( e.g. "gcc info") in work day when I on subway (another time on subway is for listening Holy Bible, reading news, or sleeping for a while). - I shall mainly forcus on finding real world issues and try to fix in time. And stop finding coding issues (which may get negative effect with others -- at least may waste other members time resources). - Still contiue for constructing all related enviroments (it is always necessary) for architectures. Welcome any ideas, suggestions or completions by any members. Thanks. -- Chen Gang Open share and attitude like air water and life which God blessed
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
After try the latest gcc again (execuse me, during these days, I have to
do another things), all things OK, the kernel can be started by QEMU,
all related issues are fixed by other members.
I shall start the make check with qemu again (it is about 12-13 days).
I guess, this thread is almost spam thread, but I still should try my
best to finish it (then I shall try to start 'make check' for tile).
By the way, if this thread really has negative effect with other members,
please warn me, I should not notify it to mailing list again, and try my
best to finish it within myself.
Thanks.
On 11/29/2014 06:25 PM, Chen Gang wrote:
> Hello Maintainers:
>
> After analysing, it is not kernel's issue, it is gcc issue, after let
> kernel related variable bypass this gcc issue, the kernel can start up
> successfully.
>
> The issue is "after declaration, the __attribute_((__section__ ...))
> will be ignored". After simplification, the related shell commands are:
>
> [root@localhost ana]# cat test.i
> extern int a;
> int a __attribute__((__section__(".data..init_task"))) = 0;
> [root@localhost ana]# /upstream/release/bin/microblaze-gchen-linux-gcc -c
> -o test.o test.i -save-temps
> [root@localhost ana]# cat test.s
> .globl a
> .bss
> .lcomm a,4,4
> .type a, @object
> [root@localhost ana]# microblaze-linux-gnu-gcc -c -o test.o test.i
> -save-temps
> [root@localhost ana]# cat test.s
> .globl a
> .section.data..init_task,"aw",@progbits
> .align 2
> .type a,@object
> .size a,4
> a:
> .space 4
> [root@localhost ana]# /upstream/release/bin/microblaze-gchen-linux-gcc -v
> Using built-in specs.
> COLLECT_GCC=/upstream/release/bin/microblaze-gchen-linux-gcc
>
> COLLECT_LTO_WRAPPER=/upstream/release/libexec/gcc/microblaze-gchen-linux/5.0.0/lto-wrapper
> Target: microblaze-gchen-linux
> Configured with: ../gcc-new/configure --target=microblaze-gchen-linux
> --disable-nls --enable-languages=c --disable-threads --disable-shared
> --with-headers=/upstream/release/kernel --disable-libssp
> --disable-libquadmath --disable-libgomp --disable-libatomic
> --with-sysroot=/upstream/release --prefix=/upstream/release : (reconfigured)
> ../gcc-new/configure --target=microblaze-gchen-linux --disable-nls
> --disable-threads --disable-shared --with-headers=/upstream/release/kernel
> --disable-libssp --disable-libquadmath --disable-libgomp --disable-libatomic
> --with-sysroot=/upstream/release --prefix=/upstream/release
> target_alias=microblaze-gchen-linux --enable-languages=c,lto --no-create
> --no-recursion
> Thread model: single
> gcc version 5.0.0 20140925 (experimental) (GCC)
>
>
> And it is fixed in the latest microblaze gcc version, but the latest gcc
> will cause another issue for compiling kernel. I shall try to analyse it
> within next month (2014-12-31), the related issue is:
>
> net/core/dev.c: In function 'register_netdevice':
> net/core/dev.c:7285:1: internal compiler error: in verify_ssa, at
> tree-ssa.c:939
>subsys_initcall(net_dev_init);
>^
> 0xbf77ab verify_ssa(bool, bool)
> ../../gcc-microblaze/gcc/tree-ssa.c:939
> 0x956e6b execute_function_todo
> ../../gcc-microblaze/gcc/passes.c:1947
> 0x95756d do_per_function
> ../../gcc-microblaze/gcc/passes.c:1639
> 0x957683 execute_todo
> ../../gcc-microblaze/gcc/passes.c:1997
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <http://gcc.gnu.org/bugs.html> for instructions.
> make[2]: *** [net/core/dev.o] Error 1
> [root@localhost ana]#
> /upstream/release-microblaze/bin/microblaze-gchen-linux-gcc -v
> Using built-in specs.
> COLLECT_GCC=/upstream/release-microblaze/bin/microblaze-gchen-linux-gcc
>
> COLLECT_LTO_WRAPPER=/upstream/release-microblaze/libexec/gcc/microblaze-gchen-linux/5.0.0/lto-wrapper
> Target: microblaze-gchen-linux
> Configured with: ../gcc-microblaze/configure
> --target=microblaze-gchen-linux --disable-nls --enable-languages=c
> --disable-threads --disable-shared
> --with-headers=/upstream/release-microblaze/kernel --disable-libssp
> --disable-libquadmath --disable-libgomp --disable-libatomic
> --with-sysroot=/upstream/release-microblaze
> --prefix=/upstream/release-microblaze
> Thread model: single
> gcc version 5.0.0 20141129 (experimental) (GCC)
>
>
> After finish analysing, I shall start "make check" under microblaze qemu
> (may need about 12-13 days for testing).
>
> Welcome any ideas, sugge
Re: [PATCH v4] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
On 11/27/14 07:41, Chen Gang wrote: > OK, I shall send related test case within two weeks (2014-12-10). > > Joseph Myers wrote: > > On Thu, 27 Nov 2014, Chen Gang wrote: > >> The original length 18 is not enough for HOST_WIDE_INT printing, need >> use 20 instead of. >> >> Also need additional bytes for printing related prefix and suffix, and >> give a related check. >> >> It passes testsuite under fedora 20 x86_64-unknown-linux-gnu. >> >> 2014-11-27 Chen Gang >> >> * c-family/c-cppbuiltin.c (builtin_define_with_int_value): Let >> buffer enough to print host wide integer value. > > OK. Properly implementing the (-9223372036854775807LL-1) and similar > cases (when the value is the least int, long or long long on the target) > can be a followup fix. > Sorry, after check all related callers, at present, it can not cause real world issue: - builtin_define_with_int_value() is static function which is only used within 'c-cppbuiltin.c'. - except builtin_define_type_sizeof(), all callers wants to use 'int' for 'value', and in fact, builtin_define_type_sizeof() uses a small integer too (although it use HOST_WIDE_INT). - for 'flag_abi_version' (can be customized by '-fabi-version=??'): '99' will over flow 16-bit 'int', but will cast to HOST_WIDE_INT automatically, which will not cause real issue. '1000 + flag_abi_version' may over flow 32-bit 'int', but still will cast to HOST_WIDE_INT automatically, which will not cause real issue. So for me, at present, current patch is not a current bug fix patch, but I guess, it is still useful in the future (the related code really need be improved). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
Hello Maintainers:
After analysing, it is not kernel's issue, it is gcc issue, after let
kernel related variable bypass this gcc issue, the kernel can start up
successfully.
The issue is "after declaration, the __attribute_((__section__ ...))
will be ignored". After simplification, the related shell commands are:
[root@localhost ana]# cat test.i
extern int a;
int a __attribute__((__section__(".data..init_task"))) = 0;
[root@localhost ana]# /upstream/release/bin/microblaze-gchen-linux-gcc -c -o
test.o test.i -save-temps
[root@localhost ana]# cat test.s
.globl a
.bss
.lcomm a,4,4
.type a, @object
[root@localhost ana]# microblaze-linux-gnu-gcc -c -o test.o test.i -save-temps
[root@localhost ana]# cat test.s
.globl a
.section.data..init_task,"aw",@progbits
.align 2
.type a,@object
.size a,4
a:
.space 4
[root@localhost ana]# /upstream/release/bin/microblaze-gchen-linux-gcc -v
Using built-in specs.
COLLECT_GCC=/upstream/release/bin/microblaze-gchen-linux-gcc
COLLECT_LTO_WRAPPER=/upstream/release/libexec/gcc/microblaze-gchen-linux/5.0.0/lto-wrapper
Target: microblaze-gchen-linux
Configured with: ../gcc-new/configure --target=microblaze-gchen-linux
--disable-nls --enable-languages=c --disable-threads --disable-shared
--with-headers=/upstream/release/kernel --disable-libssp --disable-libquadmath
--disable-libgomp --disable-libatomic --with-sysroot=/upstream/release
--prefix=/upstream/release : (reconfigured) ../gcc-new/configure
--target=microblaze-gchen-linux --disable-nls --disable-threads
--disable-shared --with-headers=/upstream/release/kernel --disable-libssp
--disable-libquadmath --disable-libgomp --disable-libatomic
--with-sysroot=/upstream/release --prefix=/upstream/release
target_alias=microblaze-gchen-linux --enable-languages=c,lto --no-create
--no-recursion
Thread model: single
gcc version 5.0.0 20140925 (experimental) (GCC)
And it is fixed in the latest microblaze gcc version, but the latest gcc
will cause another issue for compiling kernel. I shall try to analyse it
within next month (2014-12-31), the related issue is:
net/core/dev.c: In function 'register_netdevice':
net/core/dev.c:7285:1: internal compiler error: in verify_ssa, at
tree-ssa.c:939
subsys_initcall(net_dev_init);
^
0xbf77ab verify_ssa(bool, bool)
../../gcc-microblaze/gcc/tree-ssa.c:939
0x956e6b execute_function_todo
../../gcc-microblaze/gcc/passes.c:1947
0x95756d do_per_function
../../gcc-microblaze/gcc/passes.c:1639
0x957683 execute_todo
../../gcc-microblaze/gcc/passes.c:1997
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[2]: *** [net/core/dev.o] Error 1
[root@localhost ana]#
/upstream/release-microblaze/bin/microblaze-gchen-linux-gcc -v
Using built-in specs.
COLLECT_GCC=/upstream/release-microblaze/bin/microblaze-gchen-linux-gcc
COLLECT_LTO_WRAPPER=/upstream/release-microblaze/libexec/gcc/microblaze-gchen-linux/5.0.0/lto-wrapper
Target: microblaze-gchen-linux
Configured with: ../gcc-microblaze/configure --target=microblaze-gchen-linux
--disable-nls --enable-languages=c --disable-threads --disable-shared
--with-headers=/upstream/release-microblaze/kernel --disable-libssp
--disable-libquadmath --disable-libgomp --disable-libatomic
--with-sysroot=/upstream/release-microblaze
--prefix=/upstream/release-microblaze
Thread model: single
gcc version 5.0.0 20141129 (experimental) (GCC)
After finish analysing, I shall start "make check" under microblaze qemu
(may need about 12-13 days for testing).
Welcome any ideas, suggestions, and completions.
Thanks.
On 11/20/2014 11:33 PM, Chen Gang wrote:
>
> Oh, sorry, after ran more than 10 days, the qemu crashed :-(
>
> After checked the output log, and compare with the original log, we know
> we have finished more than 90% test, and it is OK (no any new issues).
> I guess the reason is I started too many other things on this machine.
>
> Next, I shall try to analyze "the cross compiled Linux kernel will run
> in dead lock" issue. After finish analyzing, I shall restart the test.
> I guess it needs 12-13 days (more than a week -- I originally expected).
>
> Thanks.
>
> On 11/9/14 21:15, Chen Gang wrote:
>>
>> At present, I use simplified sshd, ssh and scp (dropbear open source
>> program) to communicate with microblaze qemu successfully, and let gcc
>> 'make check' have real effect.
>>
>> It is just testing, at least after almost 10 hours, the log output is
>> OK. For each ssh login, it will wast 10 - 20 seconds, so I guess, the
>> "make c
Re: [PATCH v3] gcc/ubsan.c: Use 'pretty_print' for 'pretty_name' to avoid memory overflow
If necessary to add related test case, please let me know. Thanks. Send from Lenovo A788t. Jakub Jelinek wrote: >On Mon, Nov 24, 2014 at 04:28:10PM +0800, Chen Gang wrote: >> On 11/24/14 15:41, Jakub Jelinek wrote: >> > On Sun, Nov 23, 2014 at 09:13:27AM +0800, Chen Gang wrote: >> >> [...] >> >> >> + else >> >> + pp_wide_int(&pretty_name, >> >> + wi::add (wi::to_widest (TYPE_MAX_VALUE (dom)), 1), >> >> + TYPE_SIGN (TREE_TYPE (dom))); >> > >> > Space still missing before ( (and reindenting the following 2 lines). >> > >> >> Oh, thanks, if necessary to send patch v4, please let me know. > >No, just fix it up before checking in. > > Jakub
Re: [PATCH v4] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
OK, I shall send related test case within two weeks (2014-12-10). Thanks. Send from Lenovo A788t. Joseph Myers wrote: >On Thu, 27 Nov 2014, Chen Gang wrote: > >> The original length 18 is not enough for HOST_WIDE_INT printing, need >> use 20 instead of. >> >> Also need additional bytes for printing related prefix and suffix, and >> give a related check. >> >> It passes testsuite under fedora 20 x86_64-unknown-linux-gnu. >> >> 2014-11-27 Chen Gang >> >> * c-family/c-cppbuiltin.c (builtin_define_with_int_value): Let >> buffer enough to print host wide integer value. > >OK. Properly implementing the (-9223372036854775807LL-1) and similar >cases (when the value is the least int, long or long long on the target) >can be a followup fix. > >-- >Joseph S. Myers >jos...@codesourcery.com
[PATCH v4] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
The original length 18 is not enough for HOST_WIDE_INT printing, need
use 20 instead of.
Also need additional bytes for printing related prefix and suffix, and
give a related check.
It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.
2014-11-27 Chen Gang
* c-family/c-cppbuiltin.c (builtin_define_with_int_value): Let
buffer enough to print host wide integer value.
---
gcc/c-family/c-cppbuiltin.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index c571d1b..92bc06d 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -1366,14 +1366,22 @@ static void
builtin_define_with_int_value (const char *macro, HOST_WIDE_INT value)
{
char *buf;
- size_t mlen = strlen (macro);
- size_t vlen = 18;
- size_t extra = 2; /* space for = and NUL. */
-
- buf = (char *) alloca (mlen + vlen + extra);
- memcpy (buf, macro, mlen);
- buf[mlen] = '=';
- sprintf (buf + mlen + 1, HOST_WIDE_INT_PRINT_DEC, value);
+ size_t vlen = 20; /* maximize value length: -9223372036854775807 */
+ size_t extra = 6; /* space for =, NUL, (, ), and L L. */
+
+ gcc_assert (wi::fits_to_tree_p (value, long_long_integer_type_node));
+
+ buf = (char *) alloca (strlen (macro) + vlen + extra);
+
+ sprintf (buf, "%s=%s" HOST_WIDE_INT_PRINT_DEC "%s%s",
+ macro,
+ value < 0 ? "(" : "",
+ value,
+ wi::fits_to_tree_p (value, integer_type_node)
+ ? ""
+ : wi::fits_to_tree_p (value, long_integer_type_node)
+? "L" : "LL",
+ value < 0 ? ")" : "");
cpp_define (parse_in, buf);
}
--
1.9.3
Re: [PATCH v3] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
On 11/26/14 15:33, Jakub Jelinek wrote: > On Wed, Nov 26, 2014 at 09:41:16AM +0800, Chen Gang wrote: >> On 11/26/14 8:31, Joseph Myers wrote: >>> On Wed, 26 Nov 2014, Chen Gang wrote: >>> >>>> + gcc_assert (wi::fits_to_tree_p (value, char_type_node) >>>> +|| wi::fits_to_tree_p (value, short_integer_type_node) >>>> +|| wi::fits_to_tree_p (value, integer_type_node) >>>> +|| wi::fits_to_tree_p (value, long_integer_type_node) >>>> +|| wi::fits_to_tree_p (value, long_long_integer_type_node)); >>> >>> It doesn't make sense to check for char or short, since you can't write a >>> constant of one of those types. And it doesn't make sense to check for >>> int or long when checking for long long, as the ranges of int and long are >>> subsets of that of long long. So just check long long here. >>> >>>> + buf = (char *) alloca (strlen (macro) + vlen + extra); >>>> + >>>> + sprintf (buf, "%s=%s"HOST_WIDE_INT_PRINT_DEC"%s%s", > > Oh, and please use spaces around HOST_WIDE_INT_PRINT_DEC etc., > with C++11 user defined literals it is always better to have whitespace > in there. > OK, thanks, I shall notice about it when send patch v4. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH v3] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
On 11/26/14 9:38, Joseph Myers wrote: > On Wed, 26 Nov 2014, Chen Gang wrote: > >> And I have no any ideas about the attachments in your reply mail. If it >> is really related with this thread, please let me know. > > I don't understand what attachments you are referring to. > Oh, sorry, my Thunderbird mail client make a trick to me. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH v3] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
On 11/26/14 8:31, Joseph Myers wrote:
> On Wed, 26 Nov 2014, Chen Gang wrote:
>
>> + gcc_assert (wi::fits_to_tree_p (value, char_type_node)
>> + || wi::fits_to_tree_p (value, short_integer_type_node)
>> + || wi::fits_to_tree_p (value, integer_type_node)
>> + || wi::fits_to_tree_p (value, long_integer_type_node)
>> + || wi::fits_to_tree_p (value, long_long_integer_type_node));
>
> It doesn't make sense to check for char or short, since you can't write a
> constant of one of those types. And it doesn't make sense to check for
> int or long when checking for long long, as the ranges of int and long are
> subsets of that of long long. So just check long long here.
>
>> + buf = (char *) alloca (strlen (macro) + vlen + extra);
>> +
>> + sprintf (buf, "%s=%s"HOST_WIDE_INT_PRINT_DEC"%s%s",
>> + macro,
>> + value < 0 ? "(" : "",
>> + value,
>> + wi::fits_to_tree_p (value, char_type_node)
>> + || wi::fits_to_tree_p (value, short_integer_type_node)
>> + || wi::fits_to_tree_p (value, integer_type_node)
>
> No need to check for char or short here.
>
Thank you very much for your patient work, I shall send patch v4 for it
within 2 days.
And I have no any ideas about the attachments in your reply mail. If it
is really related with this thread, please let me know.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
[PATCH v3] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
The original length 18 is not enough for HOST_WIDE_INT printing, need
use 20 instead of.
Also need additional bytes for printing related prefix and suffix, and
give a related check.
It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.
2014-11-26 Chen Gang
* c-family/c-cppbuiltin.c (builtin_define_with_int_value): Let
buffer enough to print host wide integer value.
---
gcc/c-family/c-cppbuiltin.c | 30 ++
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index c571d1b..b1b96fb 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -1366,14 +1366,28 @@ static void
builtin_define_with_int_value (const char *macro, HOST_WIDE_INT value)
{
char *buf;
- size_t mlen = strlen (macro);
- size_t vlen = 18;
- size_t extra = 2; /* space for = and NUL. */
-
- buf = (char *) alloca (mlen + vlen + extra);
- memcpy (buf, macro, mlen);
- buf[mlen] = '=';
- sprintf (buf + mlen + 1, HOST_WIDE_INT_PRINT_DEC, value);
+ size_t vlen = 20; /* maximize value length: -9223372036854775807 */
+ size_t extra = 6; /* space for =, NUL, (, ), and L L. */
+
+ gcc_assert (wi::fits_to_tree_p (value, char_type_node)
+ || wi::fits_to_tree_p (value, short_integer_type_node)
+ || wi::fits_to_tree_p (value, integer_type_node)
+ || wi::fits_to_tree_p (value, long_integer_type_node)
+ || wi::fits_to_tree_p (value, long_long_integer_type_node));
+
+ buf = (char *) alloca (strlen (macro) + vlen + extra);
+
+ sprintf (buf, "%s=%s"HOST_WIDE_INT_PRINT_DEC"%s%s",
+ macro,
+ value < 0 ? "(" : "",
+ value,
+ wi::fits_to_tree_p (value, char_type_node)
+|| wi::fits_to_tree_p (value, short_integer_type_node)
+|| wi::fits_to_tree_p (value, integer_type_node)
+ ? ""
+ : wi::fits_to_tree_p (value, long_integer_type_node)
+? "L" : "LL",
+ value < 0 ? ")" : "");
cpp_define (parse_in, buf);
}
--
1.9.3
Re: [PATCH v2] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
On 11/25/14 7:56, Joseph Myers wrote:
> On Sun, 23 Nov 2014, Chen Gang wrote:
>
>> + gcc_assert (wi::fits_to_tree_p(value, integer_type_node));
>
> Watch formatting: space before '(' in the wi::fits_to_tree_p call.
> Applies elsewhere in this patch as well.
>
OK, thanks, I shall notice next.
> When making such an interface change, (a) you should update the comment on
> builtin_define_with_int_value to explain the new interface, and (b) you
> should check existing callers to make sure their values are indeed in
> range, and describe the check you did.
>
> In fact, -fabi-version=0 results in __GXX_ABI_VERSION being defined to
> 99 using builtin_define_with_int_value. That's out of range of int on
> targets with 16-bit int. So that indicates against requiring the value to
> be within range of int. It might however be OK to require the value to be
> within range of target long.
>
For me, can let builtin_define_with_int_value() fit all kinds of integer
values, and the assert need be:
gcc_assert (wi::fits_to_tree_p (value, char_type_node)
|| wi::fits_to_tree_p (value, short_integer_type_node)
|| wi::fits_to_tree_p (value, integer_type_node)
|| wi::fits_to_tree_p (value, long_integer_type_node)
|| wi::fits_to_tree_p (value, long_long_integer_type_node));
If it really can fit all kinds of integer values, for me, the related
comments of builtin_define_with_int_value() need not be changed.
>> + if (value >= 0)
>> +{
>> + sprintf (buf, "%s="HOST_WIDE_INT_PRINT_DEC"%s",
>> + macro, value,
>> + value <= HOST_INT_MAX
>> + ? ""
>> + : value <= HOST_LONG_MAX
>> + ? "L" : "LL");
>
> Limits on the host's int and long are completely irrelevant here. The
> question is the target's int and long, not the host's - and consistency
> indicates checking with wi::fits_to_tree_p (value, integer_type_node) if
> the assertion checked with long_integer_type_node.
>
OK, thanks. And for me, the related sprintf() should be:
sprintf (buf, "%s=%s"HOST_WIDE_INT_PRINT_DEC"%s%s",
macro,
value < 0 ? "(" : "",
value,
wi::fits_to_tree_p (value, char_type_node)
|| wi::fits_to_tree_p (value, short_integer_type_node)
|| wi::fits_to_tree_p (value, integer_type_node)
? ""
: wi::fits_to_tree_p (value, long_integer_type_node)
? "L" : "LL",
value < 0 ? ")" : "");
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH v2] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
OK, thank you for your work, I shall send patch v3 for it.
Send from Lenovo A788t.
Joseph Myers wrote:
>On Sun, 23 Nov 2014, Chen Gang wrote:
>
>> + gcc_assert (wi::fits_to_tree_p(value, integer_type_node));
>
>Watch formatting: space before '(' in the wi::fits_to_tree_p call.
>Applies elsewhere in this patch as well.
>
>When making such an interface change, (a) you should update the comment on
>builtin_define_with_int_value to explain the new interface, and (b) you
>should check existing callers to make sure their values are indeed in
>range, and describe the check you did.
>
>In fact, -fabi-version=0 results in __GXX_ABI_VERSION being defined to
>99 using builtin_define_with_int_value. That's out of range of int on
>targets with 16-bit int. So that indicates against requiring the value to
>be within range of int. It might however be OK to require the value to be
>within range of target long.
>
>> + if (value >= 0)
>> +{
>> + sprintf (buf, "%s="HOST_WIDE_INT_PRINT_DEC"%s",
>> + macro, value,
>> + value <= HOST_INT_MAX
>> + ? ""
>> + : value <= HOST_LONG_MAX
>> + ? "L" : "LL");
>
>Limits on the host's int and long are completely irrelevant here. The
>question is the target's int and long, not the host's - and consistency
>indicates checking with wi::fits_to_tree_p (value, integer_type_node) if
>the assertion checked with long_integer_type_node.
>
>--
>Joseph S. Myers
>jos...@codesourcery.com
Re: [PATCH v3] gcc/ubsan.c: Use 'pretty_print' for 'pretty_name' to avoid memory overflow
On 11/24/14 15:41, Jakub Jelinek wrote: > On Sun, Nov 23, 2014 at 09:13:27AM +0800, Chen Gang wrote: [...] >> + else >> +pp_wide_int(&pretty_name, >> +wi::add (wi::to_widest (TYPE_MAX_VALUE (dom)), 1), >> +TYPE_SIGN (TREE_TYPE (dom))); > > Space still missing before ( (and reindenting the following 2 lines). > Oh, thanks, if necessary to send patch v4, please let me know. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
[PATCH v3] gcc/ubsan.c: Use 'pretty_print' for 'pretty_name' to avoid memory overflow
According to the next code, 'pretty_name' may need additional bytes more
than 16 (may have unlimited length for array type). These is a easy way
for it: use 'pretty_print' for 'pretty_name'.
And not all integers are fit into tree_to_uhwi(), so also need
'wide_int' for it.
Let the code meet 2 white spaces alignment coding styles (originally,
some of code is 1 white space alignment).
It passes testsuite under fedora 20 x86)64-unknown-linux-gnu.
2014-11-23 Chen Gang
* ubsan.c (ubsan_type_descriptor): Use 'pretty_print' for
'pretty_name' to avoid memory overflow.
---
gcc/ubsan.c | 63 +
1 file changed, 34 insertions(+), 29 deletions(-)
diff --git a/gcc/ubsan.c b/gcc/ubsan.c
index b3d5343..3fceff7 100644
--- a/gcc/ubsan.c
+++ b/gcc/ubsan.c
@@ -369,7 +369,7 @@ ubsan_type_descriptor (tree type, enum ubsan_print_style
pstyle)
tree dtype = ubsan_get_type_descriptor_type ();
tree type2 = type;
const char *tname = NULL;
- char *pretty_name;
+ pretty_printer pretty_name;
unsigned char deref_depth = 0;
unsigned short tkind, tinfo;
@@ -408,54 +408,58 @@ ubsan_type_descriptor (tree type, enum ubsan_print_style
pstyle)
/* We weren't able to determine the type name. */
tname = "";
- /* Decorate the type name with '', '*', "struct", or "union". */
- pretty_name = (char *) alloca (strlen (tname) + 16 + deref_depth);
if (pstyle == UBSAN_PRINT_POINTER)
{
- int pos = sprintf (pretty_name, "'%s%s%s%s%s%s%s",
-TYPE_VOLATILE (type2) ? "volatile " : "",
-TYPE_READONLY (type2) ? "const " : "",
-TYPE_RESTRICT (type2) ? "restrict " : "",
-TYPE_ATOMIC (type2) ? "_Atomic " : "",
-TREE_CODE (type2) == RECORD_TYPE
-? "struct "
-: TREE_CODE (type2) == UNION_TYPE
- ? "union " : "", tname,
-deref_depth == 0 ? "" : " ");
+ pp_printf (&pretty_name, "'%s%s%s%s%s%s%s",
+TYPE_VOLATILE (type2) ? "volatile " : "",
+TYPE_READONLY (type2) ? "const " : "",
+TYPE_RESTRICT (type2) ? "restrict " : "",
+TYPE_ATOMIC (type2) ? "_Atomic " : "",
+TREE_CODE (type2) == RECORD_TYPE
+? "struct "
+: TREE_CODE (type2) == UNION_TYPE
+ ? "union " : "", tname,
+deref_depth == 0 ? "" : " ");
while (deref_depth-- > 0)
-pretty_name[pos++] = '*';
- pretty_name[pos++] = '\'';
- pretty_name[pos] = '\0';
+ pp_star (&pretty_name);
+ pp_quote (&pretty_name);
}
else if (pstyle == UBSAN_PRINT_ARRAY)
{
/* Pretty print the array dimensions. */
gcc_assert (TREE_CODE (type) == ARRAY_TYPE);
tree t = type;
- int pos = sprintf (pretty_name, "'%s ", tname);
+ pp_printf (&pretty_name, "'%s ", tname);
while (deref_depth-- > 0)
-pretty_name[pos++] = '*';
+ pp_star (&pretty_name);
while (TREE_CODE (t) == ARRAY_TYPE)
{
- pretty_name[pos++] = '[';
+ pp_left_bracket (&pretty_name);
tree dom = TYPE_DOMAIN (t);
if (dom && TREE_CODE (TYPE_MAX_VALUE (dom)) == INTEGER_CST)
- pos += sprintf (&pretty_name[pos], HOST_WIDE_INT_PRINT_DEC,
+ {
+ if (tree_fits_uhwi_p (TYPE_MAX_VALUE (dom))
+ && tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1 != 0)
+ pp_printf (&pretty_name, HOST_WIDE_INT_PRINT_DEC,
tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1);
+ else
+ pp_wide_int(&pretty_name,
+ wi::add (wi::to_widest (TYPE_MAX_VALUE (dom)), 1),
+ TYPE_SIGN (TREE_TYPE (dom)));
+ }
else
/* ??? We can't determine the variable name; print VLA unspec. */
- pretty_name[pos++] = '*';
- pretty_name[pos++] = ']';
+ pp_star (&pretty_name);
+ pp_right_bracket (&pretty_name);
t = TREE_TYPE (t);
}
- pretty_name[pos++] = '\'';
- pretty_name[pos] = '\0';
+ pp_quote (&pretty_name);
- /* Save the tree with stripped types.
[PATCH v2] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value
The original length 18 is not enough for HOST_WIDE_INT printing, need
use 20 instead of.
Also need additional bytes for printing related prefix and suffix, and
define the related macro in "hwint.h".
It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.
2014-11-23 Chen Gang
* c-family/c-cppbuiltin.c (builtin_define_with_int_value): Let
buffer enough to print host wide integer value.
---
gcc/c-family/c-cppbuiltin.c | 30 +++---
gcc/hwint.h | 6 ++
2 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index c571d1b..88a717b 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -1366,15 +1366,31 @@ static void
builtin_define_with_int_value (const char *macro, HOST_WIDE_INT value)
{
char *buf;
- size_t mlen = strlen (macro);
- size_t vlen = 18;
- size_t extra = 2; /* space for = and NUL. */
+ size_t vlen = 20; /* maximize value length: -9223372036854775807 */
+ size_t extra = 6; /* space for =, NUL, (, ), and L L. */
+
+ gcc_assert (wi::fits_to_tree_p(value, integer_type_node));
- buf = (char *) alloca (mlen + vlen + extra);
- memcpy (buf, macro, mlen);
- buf[mlen] = '=';
- sprintf (buf + mlen + 1, HOST_WIDE_INT_PRINT_DEC, value);
+ buf = (char *) alloca (strlen(macro) + vlen + extra);
+ if (value >= 0)
+{
+ sprintf (buf, "%s="HOST_WIDE_INT_PRINT_DEC"%s",
+ macro, value,
+ value <= HOST_INT_MAX
+ ? ""
+ : value <= HOST_LONG_MAX
+? "L" : "LL");
+}
+ else
+{
+ sprintf (buf, "%s=("HOST_WIDE_INT_PRINT_DEC"%s)",
+ macro, value,
+ value > HOST_INT_MIN
+ ? ""
+ : value > HOST_LONG_MIN
+? "L" : "LL");
+}
cpp_define (parse_in, buf);
}
diff --git a/gcc/hwint.h b/gcc/hwint.h
index fd961fd..3aecba3 100644
--- a/gcc/hwint.h
+++ b/gcc/hwint.h
@@ -225,6 +225,12 @@ exact_log2 (unsigned HOST_WIDE_INT x)
#endif /* GCC_VERSION >= 3004 */
+#define HOST_INT_MIN (int) ((unsigned int) 1 << (HOST_BITS_PER_INT - 1))
+#define HOST_INT_MAX (~(HOST_INT_MIN))
+
+#define HOST_LONG_MIN (long) ((unsigned long) 1 << (HOST_BITS_PER_LONG - 1))
+#define HOST_LONG_MAX (~(HOST_LONG_MIN))
+
#define HOST_WIDE_INT_MIN (HOST_WIDE_INT) \
((unsigned HOST_WIDE_INT) 1 << (HOST_BITS_PER_WIDE_INT - 1))
#define HOST_WIDE_INT_MAX (~(HOST_WIDE_INT_MIN))
--
1.9.3
Re: [PATCH v2] gcc/ubsan.c: Use 'pretty_print' for 'pretty_name' to avoid memory overflow
Thank you very much for your work. I shall send patch v3 for it. Thanks. On 11/22/2014 05:57 AM, Jakub Jelinek wrote: > On Sat, Nov 22, 2014 at 05:03:37AM +0800, Chen Gang wrote: >> According to the next code, 'pretty_name' may need additional bytes more >> than 16 (may have unlimited length for array type). There is an easy way >> to fix it: use 'pretty_print' for 'pretty_name'. >> >> Let the code meet 2 white spaces alignment coding styles (originally, >> some of code is 1 white sapce alignment). >> >> It passes testsuite under fedora 20 x86_64-unknown-linux-gnu. >> >> 2014-11-22 Chen Gang >> >> * ubsan.c (ubsan_type_descriptor): Use 'pretty_print' for >> 'pretty_name' to avoid memory overflow > > Add a . at the end. > >>while (deref_depth-- > 0) >> -pretty_name[pos++] = '*'; >> - pretty_name[pos++] = '\''; >> - pretty_name[pos] = '\0'; >> +pp_star(&pretty_name); >> + pp_quote(&pretty_name); > > Formatting, missing space before (. Happens many times in the patch. > >>if (dom && TREE_CODE (TYPE_MAX_VALUE (dom)) == INTEGER_CST) >> -pos += sprintf (&pretty_name[pos], HOST_WIDE_INT_PRINT_DEC, >> -tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1); >> +pp_printf (&pretty_name, HOST_WIDE_INT_PRINT_DEC, >> + tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1); > > You don't know if TYPE_MAX_VALUE (dom) will fit into uhwi, and you are > using signed printing anyway. > You said that using pp_wide_int breaks too many tests, so perhaps > do > if (tree_fits_uhwi_p (TYPE_MAX_VALUE (dom)) > && tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1 != 0) > pp_printf (..., HOST_WIDE_INT_PRINT_UNSIGNED, > else > pp_wide_int (..., wi::to_widest (TYPE_MAX_VALUE (dom)) + 1); > ? > Thanks. -- Chen Gang Open share and attitude like air water and life which God blessed
[PATCH v2] gcc/ubsan.c: Use 'pretty_print' for 'pretty_name' to avoid memory overflow
According to the next code, 'pretty_name' may need additional bytes more
than 16 (may have unlimited length for array type). There is an easy way
to fix it: use 'pretty_print' for 'pretty_name'.
Let the code meet 2 white spaces alignment coding styles (originally,
some of code is 1 white sapce alignment).
It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.
2014-11-22 Chen Gang
* ubsan.c (ubsan_type_descriptor): Use 'pretty_print' for
'pretty_name' to avoid memory overflow
---
gcc/ubsan.c | 57 +++--
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/gcc/ubsan.c b/gcc/ubsan.c
index 41cf546..c03b000 100644
--- a/gcc/ubsan.c
+++ b/gcc/ubsan.c
@@ -336,7 +336,7 @@ ubsan_type_descriptor (tree type, enum ubsan_print_style
pstyle)
tree dtype = ubsan_type_descriptor_type ();
tree type2 = type;
const char *tname = NULL;
- char *pretty_name;
+ pretty_printer pretty_name;
unsigned char deref_depth = 0;
unsigned short tkind, tinfo;
@@ -375,54 +375,50 @@ ubsan_type_descriptor (tree type, enum ubsan_print_style
pstyle)
/* We weren't able to determine the type name. */
tname = "";
- /* Decorate the type name with '', '*', "struct", or "union". */
- pretty_name = (char *) alloca (strlen (tname) + 16 + deref_depth);
if (pstyle == UBSAN_PRINT_POINTER)
{
- int pos = sprintf (pretty_name, "'%s%s%s%s%s%s%s",
-TYPE_VOLATILE (type2) ? "volatile " : "",
-TYPE_READONLY (type2) ? "const " : "",
-TYPE_RESTRICT (type2) ? "restrict " : "",
-TYPE_ATOMIC (type2) ? "_Atomic " : "",
-TREE_CODE (type2) == RECORD_TYPE
-? "struct "
-: TREE_CODE (type2) == UNION_TYPE
- ? "union " : "", tname,
-deref_depth == 0 ? "" : " ");
+ pp_printf (&pretty_name, "'%s%s%s%s%s%s%s",
+TYPE_VOLATILE (type2) ? "volatile " : "",
+TYPE_READONLY (type2) ? "const " : "",
+TYPE_RESTRICT (type2) ? "restrict " : "",
+TYPE_ATOMIC (type2) ? "_Atomic " : "",
+TREE_CODE (type2) == RECORD_TYPE
+? "struct "
+: TREE_CODE (type2) == UNION_TYPE
+ ? "union " : "", tname,
+deref_depth == 0 ? "" : " ");
while (deref_depth-- > 0)
-pretty_name[pos++] = '*';
- pretty_name[pos++] = '\'';
- pretty_name[pos] = '\0';
+ pp_star(&pretty_name);
+ pp_quote(&pretty_name);
}
else if (pstyle == UBSAN_PRINT_ARRAY)
{
/* Pretty print the array dimensions. */
gcc_assert (TREE_CODE (type) == ARRAY_TYPE);
tree t = type;
- int pos = sprintf (pretty_name, "'%s ", tname);
+ pp_printf (&pretty_name, "'%s ", tname);
while (deref_depth-- > 0)
-pretty_name[pos++] = '*';
+ pp_star(&pretty_name);
while (TREE_CODE (t) == ARRAY_TYPE)
{
- pretty_name[pos++] = '[';
+ pp_left_bracket(&pretty_name);
tree dom = TYPE_DOMAIN (t);
if (dom && TREE_CODE (TYPE_MAX_VALUE (dom)) == INTEGER_CST)
- pos += sprintf (&pretty_name[pos], HOST_WIDE_INT_PRINT_DEC,
- tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1);
+ pp_printf (&pretty_name, HOST_WIDE_INT_PRINT_DEC,
+ tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1);
else
/* ??? We can't determine the variable name; print VLA unspec. */
- pretty_name[pos++] = '*';
- pretty_name[pos++] = ']';
+ pp_star(&pretty_name);
+ pp_right_bracket(&pretty_name);
t = TREE_TYPE (t);
}
- pretty_name[pos++] = '\'';
- pretty_name[pos] = '\0';
+ pp_quote(&pretty_name);
- /* Save the tree with stripped types. */
- type = t;
+ /* Save the tree with stripped types. */
+ type = t;
}
else
-sprintf (pretty_name, "'%s'", tname);
+pp_printf (&pretty_name, "'%s'", tname);
switch (TREE_CODE (type))
{
@@ -459,8 +455,9 @@ ubsan_type_descriptor (tree type, enum ubsan_print_style
pstyle)
DECL_IGNORED_P (decl) = 1;
DECL_EXTERNAL (decl) = 0;
- size_t len = strlen (pretty_name);
- tree str = build_string (len + 1, pretty_name);
+ const char *tmp = pp_formatted_text(&pretty_name);
+ size_t len = strlen (tmp);
+ tree str = build_string (len + 1, tmp);
TREE_TYPE (str) = build_array_type (char_type_node,
build_index_type (size_int (len)));
TREE_READONLY (str) = 1;
--
1.9.3
Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length
OK, thank you for your details description.
I shall send patch v2 for it, within this week.
Send from Lenovo A788t.
Joseph Myers wrote:
>On Thu, 20 Nov 2014, Chen Gang wrote:
>
>> OK, thanks. I guess your meaning is:
>>
>> - If the value is small enough to be expressed by type 'int', we should
>>not provide 'LL'.
>
>Yes - and values not small enough should probably not be accepted by this
>function.
>
>> - If the value is positive number, we should not provide '(' and ')'.
>
>Yes.
>
>> > Really I think it
>> > would be better to require that this function is only used for values that
>> > fit in target int (given appropriate checks on all the users to make sure
>> > they fit in with that), and put a corresponding assertion there.
>> >
>>
>> Excuse me, I am not quite familiar with "target int", could you provide
>> more details for it? And if necessary, please help send patch v2 instead
>> of me:
>
>"target int" is the type int on the target, as opposed to on the host
>(with precision TYPE_PRECISION (integer_type_node)). I think you can use
>wi::fits_to_tree_p (n, integer_type_node) to test if n is in range for
>target int.
>
>--
>Joseph S. Myers
>jos...@codesourcery.com
Re: [PATCH] microblaze: microblaze.md: Use 'SI' instead of 'VOID' for operand 1 of 'call_value_intern'
Oh, sorry, after ran more than 10 days, the qemu crashed :-( After checked the output log, and compare with the original log, we know we have finished more than 90% test, and it is OK (no any new issues). I guess the reason is I started too many other things on this machine. Next, I shall try to analyze "the cross compiled Linux kernel will run in dead lock" issue. After finish analyzing, I shall restart the test. I guess it needs 12-13 days (more than a week -- I originally expected). Thanks. On 11/9/14 21:15, Chen Gang wrote: > > At present, I use simplified sshd, ssh and scp (dropbear open source > program) to communicate with microblaze qemu successfully, and let gcc > 'make check' have real effect. > > It is just testing, at least after almost 10 hours, the log output is > OK. For each ssh login, it will wast 10 - 20 seconds, so I guess, the > "make check" may run a week!! The recent operations is below: > > - zlib (for dropbear): > >export CHOST=microblaze-gchen-linux >export PATH=/upstream/release/bin:$PATH >./configure --prefix=/upstream/release && make && make install > > - dropbear (it is a simple sshd, ssh and scp): > >export PATH=/upstream/release/bin:$PATH >./configure --prefix=/upstream/release \ > --host=microblaze-gchen-linux \ > CC=microblaze-gchen-linux-gcc > >modify /ustream/release/include/stdio.h to avoid redefining sscanf to >iso99_sscanf > >link libz.a (static lib) to 'dropbear' (sshd) and 'dbclient' (ssh). >and "make scp" to generate 'scp' command. > >for supporting 'none' username: > > under ramfs, "echo 'none:x:0:0:none:/:/bin/sh' > ./etc/passwd" > >for supporting no passwords (it is temporary fix): > > modify common-session.c: "ses.authstate.pw_passwd[0] = '\0';" > >put 'dropbear', 'dbclient' and 'scp' to "./sbin" of ramfs and symbol >links to "./usr/bin" of ramfs. > >for temporary fix its stable issue, need modify code to let it 'fork' >as soon as startup, and only permit one child connection each time. > >usage: > > in microblaze qemu: > >"/sbin/dropbear -F -E -B -p 192.168.122.2:22" > > in host (x86_64), use system scp and ssh is OK (without password): > >ssh none@192.168.122.2 "cd /test; ./test" >scp test.c none@192.168.122.2:/test/ >scp none@192.168.122.2:/test/* ./ > > - For dejagnu: > >need `echo "192.168.122.2 microblaze-xilinx-gdb" >> /etc/hosts` >under /usr/local/share/dejagnu/*, use ssh/scp instead of rsh/rcp. > (need replace all, or will cause failure during make check). >for "/usr/local/share/dejagnu/baseboards/microblaze-xilinx-gdb.exp", >need add additional variables: > > set_board_info sockethost "192.168.122.2" > set_board_info username none > set timeout 600 > > Current left issues are: > > - Linux kernel built by current upstream microblaze toolchain will be >dead lock. I shall analyze it (I guess it may be kernel self issue, >which may caused by "include/compiler-gcc5.h"). > > - One patch for qemu microblaze dtb file, just checking by related >members (originally I though it was kernel issue, but after >communicate with kernel members, it is more suitable to change qemu). > > - One or more issues for dropbear (at least include stable issues), and >one or more issues for glibc. Sorry for I have to bypass them, since >I have no enough time resource on it. > > > Welcome any ideas, suggestions or completions. > > Thanks. > > > On 11/01/2014 01:07 AM, Chen Gang wrote: >> >> At present, I use telnet (without password), login to microblaze qemu >> successfully! :-) >> >> - I compile busy box with the glibc in orginal 'ramfs', so get telnetd: >>use new busybox replace the old one, and add symbol link 'telnetd' to >>busybox in "/bin". >> >> - configure qemu with network support (device "xlnx.xps-ethernetlite"). >> >>yum install libvirt >>yum install tunctl >>tunctl -b >>ip link set tap0 up >>brctl addif virbr0 tap0 >>./microblaze-softmmu/qemu-system-microblaze -M petalogix-s3adsp1800 \ >> -kernel ../linux-stable.microblaze/arch/microblaze/boot/linux.bin \ >> -no-reboot -append "console=ttyUL0,115200 doreboot" -nographic
Re: [PATCH] gcc/ubsan.c: Extend 'pretty_name' space to avoid memory overflow
On 11/17/14 18:52, Chen Gang wrote: > > What you said sound reasonable to me. > > I shall try to send patch v2 within this week (use pretty_printer). > > Thanks. > > On 11/17/14 16:15, Marek Polacek wrote: >> On Mon, Nov 17, 2014 at 08:38:19AM +0100, Jakub Jelinek wrote: >> >>> I think easiest would be to rewrite the code so that it uses pretty_printer >>> to construct the string, grep asan.c for asan_pp . Or obstacks, but you >>> don't >>> have a printer to print integers into it easily. >>> if (dom && TREE_CODE (TYPE_MAX_VALUE (dom)) == INTEGER_CST) >>> pos += sprintf (&pretty_name[pos], HOST_WIDE_INT_PRINT_DEC, >>> tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1); >>> else >>> /* ??? We can't determine the variable name; print VLA unspec. >>> */ >>> pretty_name[pos++] = '*'; >>> looks wrong anyway, as not all integers fit into uhwi. >>> Guess you could use wide_int to add 1 there and pp_wide_int. I have finish use pretty_print instead of normal sprintf, but for above case, after I tried to use wide_int, it can not pass testsuite, please help check whether what I have done is correct or not: For "make check-gcc RUNTESTFLAGS=ubsan.exp". - Simply replace is OK: pp_printf (&pretty_name, HOST_WIDE_INT_PRINT_DEC, tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1); - But use pp_wide_int for wide_int, will cause issue: pp_wide_int(&pretty_name, wi::add (wi::max_value (dom), 1), TYPE_SIGN (TREE_TYPE (dom))); - The related issues are: Running /upstream/gcc-new-x86/gcc/testsuite/gcc.dg/ubsan/ubsan.exp ... FAIL: c-c++-common/ubsan/bounds-2.c -O0 output pattern test FAIL: c-c++-common/ubsan/bounds-2.c -O1 output pattern test FAIL: c-c++-common/ubsan/bounds-2.c -O2 output pattern test FAIL: c-c++-common/ubsan/bounds-2.c -O3 -fomit-frame-pointer output pattern test FAIL: c-c++-common/ubsan/bounds-2.c -O3 -fomit-frame-pointer -funroll-loops output pattern test FAIL: c-c++-common/ubsan/bounds-2.c -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions output pattern test FAIL: c-c++-common/ubsan/bounds-2.c -O3 -g output pattern test FAIL: c-c++-common/ubsan/bounds-2.c -Os output pattern test FAIL: c-c++-common/ubsan/bounds-2.c -O2 -flto -fno-use-linker-plugin -flto-partition=none output pattern test FAIL: c-c++-common/ubsan/bounds-2.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects output pattern test FAIL: c-c++-common/ubsan/bounds-5.c -O0 output pattern test FAIL: c-c++-common/ubsan/bounds-5.c -O1 output pattern test FAIL: c-c++-common/ubsan/bounds-5.c -O2 output pattern test FAIL: c-c++-common/ubsan/bounds-5.c -O3 -fomit-frame-pointer output pattern test FAIL: c-c++-common/ubsan/bounds-5.c -O3 -fomit-frame-pointer -funroll-loops output pattern test FAIL: c-c++-common/ubsan/bounds-5.c -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions output pattern test FAIL: c-c++-common/ubsan/bounds-5.c -O3 -g output pattern test FAIL: c-c++-common/ubsan/bounds-5.c -Os output pattern test FAIL: c-c++-common/ubsan/bounds-5.c -O2 -flto -fno-use-linker-plugin -flto-partition=none output pattern test FAIL: c-c++-common/ubsan/bounds-5.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects output pattern test FAIL: c-c++-common/ubsan/bounds-7.c -O0 output pattern test FAIL: c-c++-common/ubsan/bounds-7.c -O1 output pattern test FAIL: c-c++-common/ubsan/bounds-7.c -O2 output pattern test FAIL: c-c++-common/ubsan/bounds-7.c -O3 -fomit-frame-pointer output pattern test FAIL: c-c++-common/ubsan/bounds-7.c -O3 -fomit-frame-pointer -funroll-loops output pattern test FAIL: c-c++-common/ubsan/bounds-7.c -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions output pattern test FAIL: c-c++-common/ubsan/bounds-7.c -O3 -g output pattern test FAIL: c-c++-common/ubsan/bounds-7.c -Os output pattern test FAIL: c-c++-common/ubsan/bounds-7.c -O2 -flto -fno-use-linker-plugin -flto-partition=none output pattern test FAIL: c-c++-common/ubsan/bounds-7.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects output pattern test Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length
On 11/20/14 1:20, Joseph Myers wrote:
> On Wed, 19 Nov 2014, Chen Gang wrote:
>
>> OK, thanks, what you said sounds reasonable to me. We need '(' and ')'
>> for negative members, and "LL" for the members which is larger than 32
>> bits.
>
> I don't think LL is a good idea when not needed - quite possibly some of
> the macros are expected or required to have type int.
OK, thanks. I guess your meaning is:
- If the value is small enough to be expressed by type 'int', we should
not provide 'LL'.
- If the value is positive number, we should not provide '(' and ')'.
If what I guess is correct, I shall try to send patch v2 for it within
this month.
> Really I think it
> would be better to require that this function is only used for values that
> fit in target int (given appropriate checks on all the users to make sure
> they fit in with that), and put a corresponding assertion there.
>
Excuse me, I am not quite familiar with "target int", could you provide
more details for it? And if necessary, please help send patch v2 instead
of me:
- We need provide the best code for upstream. I guess, at present, you
can, but I can't (I am not quite familiar with the related things).
- I have planned to send another v2 patches for gcc, and also analyze
some issues within gcc. I should finish them within this month (it
seems, at present, I may delay again).
- If can bear me, still let me send patch for it, I may send patch v2
in next month.
Thanks.
>> And excuse me, I do not understand why use "(-9223372036854775807LL-1)"
>> instead of "(-9223372036854775808LL)": compiler will report warning for
>> it, but for me, it is more likely the compiler's own issue:
>
> Because - and 9223372036854775808LL are separate tokens, and the latter is
> not a valid long long value.
>
OK, thanks. But for the user (e.g. a normal programmer like me), will
feel that:
- (-9223372036854775808LL) is a valid value, since it prints correctly.
- If it is reasonable to report warning for (-9223372036854775808LL),
why do not also report warning for 0x8000LL.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length
On 11/19/14 11:03, Chen Gang wrote:
> On 11/19/14 0:44, Joseph Myers wrote:
>> On Sun, 16 Nov 2014, Chen Gang wrote:
>>
>>> The maximize 64-bits integer decimal string length excluding NUL is 20 (
>>> '-9223372036854775808'), so need use 20 instead of 18 for HOST_WIDE_INT.
>>>
>>> 2014-11-16 Chen Gang
>>>
>>> * c-family/c-cppbuiltin.c (builtin_define_with_int_value): Use
>>> 20 instead of 18 for the maximize 64-bits integer decimal
>>> string length
>>
>> OK. (Though it's not a good idea to use builtin_define_with_int_value
>> with large arguments, as it won't generate any suffixes for arguments
>> outside the range of target int, and -9223372036854775808 would actually
>> need to be expressed as (-9223372036854775807LL-1). I think it's OK that
>> it doesn't parenthesize negative numbers when outputting them - that the
>> only cases for which the lack of parentheses could affect the parse are
>> invalid for other reasons - though parentheses around negative numbers
>> output might be a good idea anyway to make it obvious the output is OK,
>> and would accord with how some macros such as __*_MIN_EXP__ are output;
>> those values should probably use builtin_define_with_int_value.)
>>
>
> OK, thanks, what you said sounds reasonable to me. We need '(' and ')'
> for negative members, and "LL" for the members which is larger than 32
> bits.
>
> For me, for simplify thinking, can let all numbers have '(', ')' and
> "LL" (whether it is positive numbers or negative numbers, whether it is
> large numbers or small numbers), and one sprintf() is enough:
>
> sprintf(buf, "%s=("HOST_WIDE_INT_PRINT_DEC")LL", macro, value);
Oh, sorry, need be:
sprintf(buf, "%s=("HOST_WIDE_INT_PRINT_DEC"LL)", macro, value);
>
> And excuse me, I do not understand why use "(-9223372036854775807LL-1)"
> instead of "(-9223372036854775808LL)": compiler will report warning for
> it, but for me, it is more likely the compiler's own issue:
>
> bash-3.2# cat ./test.c
> #include
>
> int main ()
> {
> long long i = (-9223372036854775808LL);
> long long j = 0x8000LL;
> long long k = 0x7fffLL;
>
> printf("\ni: %lld, j: %lld, k: %lld\n", i, j, k);
>
> return 0;
> }
> bash-3.2# gcc -o test test.c
> test.c:5:19: warning: integer constant is larger than the largest signed
> integer type
> long long i = (-9223372036854775808LL);
> ^
> 1 warning generated.
> bash-3.2# ./test
>
> i: -9223372036854775808, j: -9223372036854775808, k: 9223372036854775807
>
>
>
> Thanks
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length
On 11/19/14 0:44, Joseph Myers wrote:
> On Sun, 16 Nov 2014, Chen Gang wrote:
>
>> The maximize 64-bits integer decimal string length excluding NUL is 20 (
>> '-9223372036854775808'), so need use 20 instead of 18 for HOST_WIDE_INT.
>>
>> 2014-11-16 Chen Gang
>>
>> * c-family/c-cppbuiltin.c (builtin_define_with_int_value): Use
>> 20 instead of 18 for the maximize 64-bits integer decimal
>> string length
>
> OK. (Though it's not a good idea to use builtin_define_with_int_value
> with large arguments, as it won't generate any suffixes for arguments
> outside the range of target int, and -9223372036854775808 would actually
> need to be expressed as (-9223372036854775807LL-1). I think it's OK that
> it doesn't parenthesize negative numbers when outputting them - that the
> only cases for which the lack of parentheses could affect the parse are
> invalid for other reasons - though parentheses around negative numbers
> output might be a good idea anyway to make it obvious the output is OK,
> and would accord with how some macros such as __*_MIN_EXP__ are output;
> those values should probably use builtin_define_with_int_value.)
>
OK, thanks, what you said sounds reasonable to me. We need '(' and ')'
for negative members, and "LL" for the members which is larger than 32
bits.
For me, for simplify thinking, can let all numbers have '(', ')' and
"LL" (whether it is positive numbers or negative numbers, whether it is
large numbers or small numbers), and one sprintf() is enough:
sprintf(buf, "%s=("HOST_WIDE_INT_PRINT_DEC")LL", macro, value);
And excuse me, I do not understand why use "(-9223372036854775807LL-1)"
instead of "(-9223372036854775808LL)": compiler will report warning for
it, but for me, it is more likely the compiler's own issue:
bash-3.2# cat ./test.c
#include
int main ()
{
long long i = (-9223372036854775808LL);
long long j = 0x8000LL;
long long k = 0x7fffLL;
printf("\ni: %lld, j: %lld, k: %lld\n", i, j, k);
return 0;
}
bash-3.2# gcc -o test test.c
test.c:5:19: warning: integer constant is larger than the largest signed
integer type
long long i = (-9223372036854775808LL);
^
1 warning generated.
bash-3.2# ./test
i: -9223372036854775808, j: -9223372036854775808, k: 9223372036854775807
Thanks
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] Check 'fd' neither -1 nor 0, before close it
On 11/18/14 0:38, Joseph Myers wrote: > On Sat, 15 Nov 2014, Chen Gang wrote: > >> Also in c_common_read_pch(), when failure occurs, also need be sure the >> 'fd' is not '-1' for the next close operation. > > Please clarify how c_common_read_pch gets called with fd == -1. > Certainly checking after an error is too late; we shouldn't call fdopen in > that case at all, and I think something further up the call chain should > have avoided calling c_common_read_pch with fd == -1 at all (the question > is just exactly what point on the call chain is missing such a check and > needs it added). > According to current source code, the author wants 'fd' should never be '-1' in c_common_read_pch(). "grep -rn read_pch *" in root directory, then "grep -rn _cpp_stack_file *", can know it is used in 3 areas: - c_common_pch_pragma() in "gcc/c-family/c-pch.c", it has already checked 'fd' before call c_common_read_pch() - _cpp_stack_include() in "libcpp/files.c", before _cpp_stack_file(), has called and checked _cpp_find_file(). - cpp_read_main_file() in "libcpp/init.c", before _cpp_stack_file(), has called and checked _cpp_find_file(). In c_common_read_pch(), even if 'fd' is '-1', we should check it firstly before call fdopen(), instead of check '-1' in failure code block. But for me, it contents another 2 related issues: - _cpp_find_file() always return a valid pointer, so related check code "file == NULL" is no use for _cpp_find_file() in _cpp_stack_include(). - According to its comments, _cpp_find_file() can not be sure of 'file->fd' must not be '-1', even when "file->err_no == 0", we need reopen it if necessary. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [PATCH] gcc/ubsan.c: Extend 'pretty_name' space to avoid memory overflow
What you said sound reasonable to me.
I shall try to send patch v2 within this week (use pretty_printer).
Thanks.
On 11/17/14 16:15, Marek Polacek wrote:
> On Mon, Nov 17, 2014 at 08:38:19AM +0100, Jakub Jelinek wrote:
>> On Mon, Nov 17, 2014 at 08:16:32AM +0100, Marek Polacek wrote:
>>> On Mon, Nov 17, 2014 at 06:40:26AM +0800, Chen Gang wrote:
>>>> According to the next code, 'pretty_name' may need additional bytes more
>>>> than 16. For simplify thinking and being extensible in future, extent it
>>>> to 256 bytes, directly.
>>>
>>> I think + 128 bytes should be enough for everyone.
>>
>> I disagree.
>> Consider:
>> typedef char
>> A[1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1]
[
1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1];
>> A a;
>>
>> int foo (int j)
>> {
>>
>> a[j][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0
]
[0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0]
= 1;
>> }
>> You need 1159 bytes in that case. Easily to construct testcase that needs
>> arbitrary amount.
>
> Ah, I haven't looked at the UBSAN_PRINT_ARRAY case.
>
>> I think easiest would be to rewrite the code so that it uses pretty_printer
>> to construct the string, grep asan.c for asan_pp . Or obstacks, but you
>> don't
>> have a printer to print integers into it easily.
>> if (dom && TREE_CODE (TYPE_MAX_VALUE (dom)) == INTEGER_CST)
>> pos += sprintf (&pretty_name[pos], HOST_WIDE_INT_PRINT_DEC,
>> tree_to_uhwi (TYPE_MAX_VALUE (dom)) + 1);
>> else
>> /* ??? We can't determine the variable name; print VLA unspec.
>> */
>> pretty_name[pos++] = '*';
>> looks wrong anyway, as not all integers fit into uhwi.
>> Guess you could use wide_int to add 1 there and pp_wide_int.
>
> Ok.
>
> Marek
>
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
[PATCH] gcc/ubsan.c: Extend 'pretty_name' space to avoid memory overflow
According to the next code, 'pretty_name' may need additional bytes more
than 16. For simplify thinking and being extensible in future, extent it
to 256 bytes, directly.
It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.
2014-11-17 Chen Gang
* ubsan.c (ubsan_type_descriptor): Extend 'pretty_name' space to
avoid memory overflow.
---
gcc/ubsan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/ubsan.c b/gcc/ubsan.c
index 41cf546..12b05cd 100644
--- a/gcc/ubsan.c
+++ b/gcc/ubsan.c
@@ -376,7 +376,7 @@ ubsan_type_descriptor (tree type, enum ubsan_print_style
pstyle)
tname = "";
/* Decorate the type name with '', '*', "struct", or "union". */
- pretty_name = (char *) alloca (strlen (tname) + 16 + deref_depth);
+ pretty_name = (char *) alloca (strlen (tname) + 256 + deref_depth);
if (pstyle == UBSAN_PRINT_POINTER)
{
int pos = sprintf (pretty_name, "'%s%s%s%s%s%s%s",
--
1.9.3
Re: [PATCH] gcc/c-family/c-cppbuiltin.c: Add two bytes for avoiding memory overflow issue
Oh, sorry, it is incorrect, original code is already add 2 bytes for it.
Thanks.
On 11/16/2014 10:32 PM, Chen Gang wrote:
> When 'is_str' is true, need consider about 2 '"' for the extra space, or
> will cause memory overflow.
>
> 2014-11-16 Chen Gang
>
> * c-family/c-cppbuiltin.c (builtin_define_with_value): Add two
> bytes for avoiding memory overflow issue.
> ---
> gcc/c-family/c-cppbuiltin.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
> index 8e8cec4..cc3d90b 100644
> --- a/gcc/c-family/c-cppbuiltin.c
> +++ b/gcc/c-family/c-cppbuiltin.c
> @@ -1282,7 +1282,7 @@ builtin_define_with_value (const char *macro, const
> char *expansion, int is_str)
>char *buf;
>size_t mlen = strlen (macro);
>size_t elen = strlen (expansion);
> - size_t extra = 2; /* space for an = and a NUL */
> + size_t extra = 4; /* space for an =, a NUL, and 2 '"' when is_str is true
> */
>
>if (is_str)
> {
>
--
Chen Gang
Open share and attitude like air water and life which God blessed
[PATCH] gcc/c-family/c-cppbuiltin.c: Add two bytes for avoiding memory overflow issue
When 'is_str' is true, need consider about 2 '"' for the extra space, or
will cause memory overflow.
2014-11-16 Chen Gang
* c-family/c-cppbuiltin.c (builtin_define_with_value): Add two
bytes for avoiding memory overflow issue.
---
gcc/c-family/c-cppbuiltin.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index 8e8cec4..cc3d90b 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -1282,7 +1282,7 @@ builtin_define_with_value (const char *macro, const char
*expansion, int is_str)
char *buf;
size_t mlen = strlen (macro);
size_t elen = strlen (expansion);
- size_t extra = 2; /* space for an = and a NUL */
+ size_t extra = 4; /* space for an =, a NUL, and 2 '"' when is_str is true */
if (is_str)
{
--
1.9.3
[PATCH] gcc/c-family/c-cppbuiltin.c: Use 20 instead of 18 for the maximized 64-bits integer decimal string length
The maximize 64-bits integer decimal string length excluding NUL is 20 (
'-9223372036854775808'), so need use 20 instead of 18 for HOST_WIDE_INT.
2014-11-16 Chen Gang
* c-family/c-cppbuiltin.c (builtin_define_with_int_value): Use
20 instead of 18 for the maximize 64-bits integer decimal
string length
---
gcc/c-family/c-cppbuiltin.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index 26fabc2..8e8cec4 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -1345,7 +1345,7 @@ builtin_define_with_int_value (const char *macro,
HOST_WIDE_INT value)
{
char *buf;
size_t mlen = strlen (macro);
- size_t vlen = 18;
+ size_t vlen = 20; /* maximize value length: -9223372036854775808 */
size_t extra = 2; /* space for = and NUL. */
buf = (char *) alloca (mlen + vlen + extra);
--
1.9.3
