Re: [patch,avr] Fix PR80929, work around middle-end PR81444
2017-07-14 18:16 GMT+04:00 Georg-Johann Lay : > Hi, this patch restores some of the divmod handling. > > It addresses two issues: > > 1) rtx_costs might be called with code = LSHIFTRT for a mul_highpart. This > is the case when outer_code = TRUNCATE. This patch uses a new function to > compute the costs for that case (also used for code = TRUNCATE). > > 2) Due to PR81444, the middle-end is calling the cost functions with strange > RTXes like > > (truncate:HI (lshiftrt:PSI (mult:PSI (zero_extend:PSI (reg:TI 42)) >(zero_extend:PSI (reg:TI 42))) > (const_int 16 [0x10]))) > > i.e. completely messed up modes. The correct RTX would be > > (truncate:HI (lshiftrt:SI (mult:SI (zero_extend:SI (reg:HI 42)) > (zero_extend:SI (reg:HI 42))) > (const_int 16 [0x10]))) > > The is worked around by also considering PSImode in the cost computation. > > FYI, I am currently also just reg-testing a fix for PR81444, cf. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81444#c1 > > Ok to apply the patch below (trunk and v7)? > > Johann > > PR 80929 > * config/avr/avr.c (avr_mul_highpart_cost): New static function. > (avr_rtx_costs_1) [TRUNCATE]: Use it to compute mul_highpart cost. > [LSHIFTRT, outer_code = TRUNCATE]: Same. Approved. Please apply.
Re: [PATCH/AARCH64] Decimal floating point support for AARCH64
On Thu, Jul 13, 2017 at 5:12 PM, Andrew Pinski wrote:
> Hi,
> This patch adds Decimal floating point support to aarch64. It is
> the base support in that since there is no hardware support for DFP,
> it just defines the ABI. The ABI I chose is that _Decimal32 is
> treated like float, _Decimal64 is treated like double and _Decimal128
> is treated like long double. In that they are passed via the floating
> registers (sN, dN, qN).
> Is this ok an ABI?
>
> Is the patch ok? Bootstrapped and tested on aarch64-linux-gnu with
> --enable-decimal-float with no regressions and all of the dfp
> testcases pass.
I just noticed I messed up by attaching the wrong patch.
Here is the correct one.
Thanks,
Andrew
>
> Thanks,
> Andrew Pinski
>
> gcc/ChangeLog:
> * config/aarch64/aarch64.c (aarch64_split_128bit_move): Handle TDmode.
> (aarch64_classify_address): Likewise.
> (aarch64_legitimize_address_displacement): Likewise.
> (aarch64_legitimize_address): Likewise.
> (aarch64_constant_pool_reload_icode): Handle SD, DD, and TD modes.
> (aarch64_secondary_reload): Handle TDmode.
> (aarch64_valid_floating_const): For decimal floating point return false.
> (aarch64_gimplify_va_arg_expr): Handle SD, DD, and TD modes.
> (aapcs_vfp_sub_candidate): Likewise.
> (aarch64_vfp_is_call_or_return_candidate): Handle MODE_DECIMAL_FLOAT.
> (aarch64_scalar_mode_supported_p): For DECIMAL_FLOAT_MODE_P, return
> default_decimal_float_supported_p.
> * config/aarch64/iterators.md (GPF_TF_F16): Add SD, DD, and TD modes.
> (SFD): New iterator.
> (DFD): New iterator.
> (TFD): New iterator.
> (GPF_TF): Add SD, DD, and TD modes.
> (TX): Add TD mode.
> * config/aarch64/aarch64.md (*movsf_aarch64): Use SFD iterator.
> (*movdf_aarch64): Use DFD iterator.
> (*movtf_aarch64): Use TFD iterator.
> (define_split for TF): Use TFD iterator.
>
>
> gcc/testsuite/ChangeLog:
> * c-c++-common/dfp/pr39986.c: Allow for word instead of just long.
>
> libgcc/ChangeLog:
> * config.host (aarch64*-*-elf): Add t-dfprules to tmake_file.
> (aarch64*-*-freebsd*): Likewise.
> (aarch64*-*-linux*): Likewise.
Index: gcc/config/aarch64/aarch64.c
===
--- gcc/config/aarch64/aarch64.c(revision 250186)
+++ gcc/config/aarch64/aarch64.c(working copy)
@@ -1653,7 +1653,7 @@ aarch64_split_128bit_move (rtx dst, rtx
machine_mode mode = GET_MODE (dst);
- gcc_assert (mode == TImode || mode == TFmode);
+ gcc_assert (mode == TImode || mode == TFmode || mode == TDmode);
gcc_assert (!(side_effects_p (src) || side_effects_p (dst)));
gcc_assert (mode == GET_MODE (src) || GET_MODE (src) == VOIDmode);
@@ -1673,11 +1673,16 @@ aarch64_split_128bit_move (rtx dst, rtx
emit_insn (gen_aarch64_movtilow_di (dst, src_lo));
emit_insn (gen_aarch64_movtihigh_di (dst, src_hi));
}
- else
+ else if (mode == TFmode)
{
emit_insn (gen_aarch64_movtflow_di (dst, src_lo));
emit_insn (gen_aarch64_movtfhigh_di (dst, src_hi));
}
+ else
+ {
+ emit_insn (gen_aarch64_movtdlow_di (dst, src_lo));
+ emit_insn (gen_aarch64_movtdhigh_di (dst, src_hi));
+ }
return;
}
else if (GP_REGNUM_P (dst_regno) && FP_REGNUM_P (src_regno))
@@ -1690,11 +1695,16 @@ aarch64_split_128bit_move (rtx dst, rtx
emit_insn (gen_aarch64_movdi_tilow (dst_lo, src));
emit_insn (gen_aarch64_movdi_tihigh (dst_hi, src));
}
- else
+ else if (mode == TFmode)
{
emit_insn (gen_aarch64_movdi_tflow (dst_lo, src));
emit_insn (gen_aarch64_movdi_tfhigh (dst_hi, src));
}
+ else if (mode == TDmode)
+ {
+ emit_insn (gen_aarch64_movdi_tdlow (dst_lo, src));
+ emit_insn (gen_aarch64_movdi_tdhigh (dst_hi, src));
+ }
return;
}
}
@@ -4420,10 +4430,11 @@ aarch64_classify_address (struct aarch64
rtx op0, op1;
/* On BE, we use load/store pair for all large int mode load/stores.
- TI/TFmode may also use a load/store pair. */
+ TI/TF/TDmode may also use a load/store pair. */
bool load_store_pair_p = (outer_code == PARALLEL
|| mode == TImode
|| mode == TFmode
+ || mode == TDmode
|| (BYTES_BIG_ENDIAN
&& aarch64_vect_struct_mode_p (mode)));
@@ -4473,7 +4484,7 @@ aarch64_classify_address (struct aarch64
info->base = op0;
info->offset = op1;
- /* TImode and TFmode values are allowed in both pairs of X
+ /* TImode and TFmode and TDmode values are allowed in both pairs of X
registers and individual Q registers. The available
address modes are:
X,X: 7-bit signed scaled offset
@@ -4482,7 +4493
Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
On Fri, Jul 14, 2017 at 12:59 PM, Jim Wilson wrote: > On Fri, Jul 14, 2017 at 1:35 AM, Martin Liška wrote: >> May I ask Jim to test the patch? >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > I started an aarch64 bootstrap to test. My fast machine is busy with > work tasks, so I have to use a slower machine, and hence this will > take some time. The aarch64-linux bootstrap with the patch succeeds. I also did a make check, and see no regressions. Jim
Re: Add support to trace comparison instructions and switch statements
On Fri, Jul 14, 2017 at 11:17 PM, Kostya Serebryany wrote:
> Hi
>
> I wrote a test for "-fsanitize-coverage=trace-cmp" .
>
> Is there anybody tells me if these codes could be merged into gcc ?
Nice!
We are currently working on Linux kernel fuzzing that use the
comparison tracing. We use clang at the moment, but having this
support in gcc would be great for kernel land.
One concern I have: do we want to do some final refinements to the API
before we implement this in both compilers?
2 things we considered from our perspective:
- communicating to the runtime which operands are constants
- communicating to the runtime which comparisons are counting loop checks
First is useful if you do "find one operand in input and replace with
the other one" thing. Second is useful because counting loop checks
are usually not useful (at least all but one).
In the original Go implementation I also conveyed signedness of
operands, exact comparison operation (<, >, etc):
https://github.com/dvyukov/go-fuzz/blob/master/go-fuzz-defs/defs.go#L13
But I did not find any use for that.
I also gave all comparisons unique IDs:
https://github.com/dvyukov/go-fuzz/blob/master/go-fuzz-dep/sonar.go#L24
That turned out to be useful. And there are chances we will want this
for C/C++ as well.
Kostya, did anything like this pop up in your work on libfuzzer?
Can we still change the clang API? At least add an additional argument
to the callbacks?
>>>
>>>
>>> I'd prefer not to change the API, but extend it (new compiler flag, new
>>> callbacks), if absolutely needed.
>>> Probably make it trace-cmp-guard (similar to trace-pc-guard, with an extra
>>> parameter that has the ID).
>>> I don't like the approach with compiler-generated constant IDs.
>>
>> Yes, if we do it for C/C++, we need to create globals and pass pointer
>> to a global to the callbacks. IDs do not work for C/C++.
>>
>>> Yes, it's a bit more efficient, but much less flexible in presence of
>>> multiple modules, DSOs, dlopen, etc.
>>>
>>> I was also looking at completely inlining this instrumentation because it's
>>> pretty expensive even in it's current form
>>> (adding more parameters will make things worse).
>>> This is going to be much less flexible, of course, so I'll attack it only
>>> once I settle on the algorithm to handle CMPs in libFuzzer.
>>
>> This will require a new, completely different API for
>> compiler<->runtime anyway, so we can put this aside for now.
>>
>>
At the very least I would suggest that we add an additional arg that
contains some flags (1/2 arg is a const, this is counting loop check,
etc). If we do that we can also have just 1 callback that accepts
uint64's for args because we can pass operand size in the flags:
>>>
>>>
>>> How many flag combinations do we need and do we *really* need them?
>>>
>>> If the number of flag combinations is small, I'd prefer to have separate
>>> callbacks (__sanitizer_cov_trace_cmp_loop_bound ?)
>>>
>>> Do we really need to know that one arg is a const?
>>> It could well be a constant in fact, but compiler won't see it.
>>>
>>> int foo(int n) { ... if (i < n) ... }
>>> ...
>>> foo(42); // not inlined.
>>>
>>> We need to handle both cases the same way.
>>
>>
>> Well, following this line of reasoning we would need only
>> __asan_load/storeN callbacks for asan and remove
>> __asan_load/store1/2/4/8, because compiler might not know the size at
>> compile time. Constant-ness is only an optimization. If compiler does
>> not know that something is a const, fine. Based on my experience with
>> go-fuzz and our early experience with kernel, we badly need const
>> hint.
>> Otherwise fuzzer generates gazillions of candidates based on
>> comparison arguments. Note that kernel is several order of magnitude
>> larger than what people usually fuzz in user-space, inputs are more
>> complex and at the same time execution speed is several order of
>> magnitude lower. We can't rely on raw speed.
>>
>> Thinking of this more, I don't thing that globals will be useful in
>> the kernel context (the main problem is that we have multiple
>> transient isolated kernels). If we track per-comparison site
>> information, we will probably use PCs. So I am ready to give up on
>> this.
>>
>> Both of you expressed concerns about performance. Kostya says we
>> should not break existing clang API.
>> If we limit this to only constant-ness, then I think we can make this
>> both forward and backward compatible, which means we don't need to
>> handle it now. E.g. we can:
>> - if both operands are const (if it's possible at all), don't emit any
>> callback
>> - if only one is const, emit __sanitizer_cov_trace_cmp_const1 and
>> pass the const in a known position (i.e. always first/second arg)
>> - if none are const, emit callback __sanitizer_cov_trace_cmp_dyn1
>> Then
Re: [PATCH] Kill TYPE_METHODS rtl 3/9
On 07/14/2017 10:54 AM, Nathan Sidwell wrote: > This was the most surprising check of TYPE_METHODS. When not optimizing > we use the non-nullness of TYPE_METHODS to figure out if we want to > place a non BLKmode structure into a register. On the grounds that one > can't call a member function with a register-located object. > > That seems overly enthusiastic -- if we're not optimizing, who cares? > > (When we zap TYHPE_METHODS we currently set it to error_mark_node, if it > was non-null, so that this above check will work). > > I'd appreciate comment. Definitely a surprise. I was hatching an idea this might be related to the old stupid.c allocator, but it turns out this code is much newer than that. If I'm reading BZ39485 correctly, the issue is that when not optimizing, we were shoving the value into a register rather than dumping it into memory. As a result gdb was unable to do an inferior call to its the object's methods. Egad. jeff
libgo patch committed: Don't copy semt into runtime.inc
PR 81449 reports a libgo problem with the definition semt in
runtime.inc on some systems. Since the C code in libgo/runtime doesn't
need semt, just don't copy it into runtime.inc. Bootstrapped and ran
Go testsuite on x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 250216)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-a77b1091fa83cfaf200e6dad26e92e42483bae43
+2ae6bf76f97f7d4c63a1f0ad0683b9ba62baaf06
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: libgo/Makefile.am
===
--- libgo/Makefile.am (revision 249799)
+++ libgo/Makefile.am (working copy)
@@ -543,6 +543,8 @@ s-sigtab: $(srcdir)/mksigtab.sh gen-sysi
# _Complex_lock and _Reader_lock are Go translations of some AIX system
# types and should not be exported back to C
+# semt is a Go translation of the C type sem_t; it fails to convert on
+# some systems and need not be exported back to C.
# sigset conflicts with system type sigset on AIX, so we need to rename it
runtime.inc: s-runtime-inc; @true
s-runtime-inc: runtime.lo Makefile
@@ -551,7 +553,7 @@ s-runtime-inc: runtime.lo Makefile
for pattern in '_[GP][a-z]' _Max _Lock _Sig _Trace _MHeap _Num; do \
grep "#define $$pattern" runtime.inc.tmp >> runtime.inc.tmp2; \
done
- for TYPE in _Complex_lock _Reader_lock; do \
+ for TYPE in _Complex_lock _Reader_lock semt; do \
sed -e '/struct '$${TYPE}' {/,/^}/s/^.*$$//' runtime.inc.tmp2 >
runtime.inc.tmp3; \
mv runtime.inc.tmp3 runtime.inc.tmp2; \
done
Re: gotools patch committed: Test runtime, misc/cgo/{test,testcarchive}
On Fri, Jul 14, 2017 at 11:41 AM, Uros Bizjak wrote: > On Thu, Jul 13, 2017 at 9:56 PM, Ian Lance Taylor wrote: >> On Thu, Jun 29, 2017 at 11:40 PM, Uros Bizjak wrote: >>> This patch to the gotools Makefile adds tests to `make check`. We now test the runtime package using the newly built go tool, and test that cgo works by running the misc/cgo/test and misc/cgo/testcarchive tests. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline. >>> >>> There are several failures on non-split targets, e.g.: >>> >>> FAIL: TestCgoHandlesWlORIGIN >>> go_test.go:267: running testgo [build origin] >>> go_test.go:286: standard error: >>> go_test.go:287: # origin >>> cc1: error: '-fsplit-stack' requires assembler support for CFI >>> directives >>> cc1: error: '-fsplit-stack' is not supported by this compiler >>> configuration >>> >>> and: >>> >>> FAIL: TestCgoCrashHandler >>> crash_test.go:70: building testprogcgo []: exit status 2 >>> # _/home/uros/git/gcc/libgo/go/runtime/testdata/testprogcgo >>> cc1: error: '-fsplit-stack' requires assembler support for CFI >>> directives >>> cc1: error: '-fsplit-stack' is not supported by this compiler >>> configuration >>> >>> As evident from TestBuildDryRunWithCgo dump, -fsplit-stack argument is >>> added unconditionally to the compile flags. >> >> Would you mind checking whether this patch fixes the problem on your >> system? Thanks. > > Yes, this patch correctly detects that the compiler doesn't support > -fsplit-stack. Thanks for checking. I've committed the patch to mainline. Ian
Re: Add support to trace comparison instructions and switch statements
On Fri, Jul 14, 2017 at 5:23 AM, Dmitry Vyukov wrote:
> On Thu, Jul 13, 2017 at 11:18 PM, Kostya Serebryany wrote:
>>> > Hi
>>> >
>>> > I wrote a test for "-fsanitize-coverage=trace-cmp" .
>>> >
>>> > Is there anybody tells me if these codes could be merged into gcc ?
>>>
>>>
>>> Nice!
>>>
>>> We are currently working on Linux kernel fuzzing that use the
>>> comparison tracing. We use clang at the moment, but having this
>>> support in gcc would be great for kernel land.
>>>
>>> One concern I have: do we want to do some final refinements to the API
>>> before we implement this in both compilers?
>>>
>>> 2 things we considered from our perspective:
>>> - communicating to the runtime which operands are constants
>>> - communicating to the runtime which comparisons are counting loop checks
>>>
>>> First is useful if you do "find one operand in input and replace with
>>> the other one" thing. Second is useful because counting loop checks
>>> are usually not useful (at least all but one).
>>> In the original Go implementation I also conveyed signedness of
>>> operands, exact comparison operation (<, >, etc):
>>> https://github.com/dvyukov/go-fuzz/blob/master/go-fuzz-defs/defs.go#L13
>>> But I did not find any use for that.
>>> I also gave all comparisons unique IDs:
>>> https://github.com/dvyukov/go-fuzz/blob/master/go-fuzz-dep/sonar.go#L24
>>> That turned out to be useful. And there are chances we will want this
>>> for C/C++ as well.
>>>
>>> Kostya, did anything like this pop up in your work on libfuzzer?
>>> Can we still change the clang API? At least add an additional argument
>>> to the callbacks?
>>
>>
>> I'd prefer not to change the API, but extend it (new compiler flag, new
>> callbacks), if absolutely needed.
>> Probably make it trace-cmp-guard (similar to trace-pc-guard, with an extra
>> parameter that has the ID).
>> I don't like the approach with compiler-generated constant IDs.
>
> Yes, if we do it for C/C++, we need to create globals and pass pointer
> to a global to the callbacks. IDs do not work for C/C++.
>
>> Yes, it's a bit more efficient, but much less flexible in presence of
>> multiple modules, DSOs, dlopen, etc.
>>
>> I was also looking at completely inlining this instrumentation because it's
>> pretty expensive even in it's current form
>> (adding more parameters will make things worse).
>> This is going to be much less flexible, of course, so I'll attack it only
>> once I settle on the algorithm to handle CMPs in libFuzzer.
>
> This will require a new, completely different API for
> compiler<->runtime anyway, so we can put this aside for now.
>
>
>>> At the very least I would suggest that we add an additional arg that
>>> contains some flags (1/2 arg is a const, this is counting loop check,
>>> etc). If we do that we can also have just 1 callback that accepts
>>> uint64's for args because we can pass operand size in the flags:
>>
>>
>> How many flag combinations do we need and do we *really* need them?
>>
>> If the number of flag combinations is small, I'd prefer to have separate
>> callbacks (__sanitizer_cov_trace_cmp_loop_bound ?)
>>
>> Do we really need to know that one arg is a const?
>> It could well be a constant in fact, but compiler won't see it.
>>
>> int foo(int n) { ... if (i < n) ... }
>> ...
>> foo(42); // not inlined.
>>
>> We need to handle both cases the same way.
>
>
> Well, following this line of reasoning we would need only
> __asan_load/storeN callbacks for asan and remove
> __asan_load/store1/2/4/8, because compiler might not know the size at
> compile time. Constant-ness is only an optimization. If compiler does
> not know that something is a const, fine. Based on my experience with
> go-fuzz and our early experience with kernel, we badly need const
> hint.
> Otherwise fuzzer generates gazillions of candidates based on
> comparison arguments. Note that kernel is several order of magnitude
> larger than what people usually fuzz in user-space, inputs are more
> complex and at the same time execution speed is several order of
> magnitude lower. We can't rely on raw speed.
>
> Thinking of this more, I don't thing that globals will be useful in
> the kernel context (the main problem is that we have multiple
> transient isolated kernels). If we track per-comparison site
> information, we will probably use PCs. So I am ready to give up on
> this.
>
> Both of you expressed concerns about performance. Kostya says we
> should not break existing clang API.
> If we limit this to only constant-ness, then I think we can make this
> both forward and backward compatible, which means we don't need to
> handle it now. E.g. we can:
> - if both operands are const (if it's possible at all), don't emit any
> callback
> - if only one is const, emit __sanitizer_cov_trace_cmp_const1 and
> pass the const in a known position (i.e. always first/second arg)
> - if none are const, emit callback __sanitizer_cov_trace_cmp_dyn1
> Then compiler emits weak aliases form
> __sanitizer_cov_t
Re: [PATCH, rs6000] Add testcases for vec_cnttz
Hi Carl,
On Fri, Jul 14, 2017 at 01:20:32PM -0700, Carl Love wrote:
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-4-p9-runnable.c
> @@ -0,0 +1,95 @@
> +/* { dg-do run { target { powerpc64*-*-* && { lp64 && p9vector_hw } } } } */
powerpc*-*-* instead.
> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
> "-mcpu=power9" } } */
> +/* { dg-options "-mcpu=power9 -O2 -mupper-regs-di" } */
Does this need to be restricted to 64-bit? Please test.
Okay modulo those nits, thanks!
Segher
[PATCH, rs6000] Add testcases for vec_cnttz
GCC Maintainers:
The following patch add testcases and documentation for the
---
gcc/ChangeLog:
2017-07-14 Carl Love
* doc/extend.texi: Update the built-in documentation file for the
existing built-in functions
vector signed char vec_cnttz (vector signed char);
vector unsigned char vec_cnttz (vector unsigned char);
vector signed short vec_cnttz (vector signed short);
vector unsigned short vec_cnttz (vector unsigned short);
vector signed int vec_cnttz (vector signed int);
vector unsigned int vec_cnttz (vector unsigned int);
vector signed long long vec_cnttz (vector signed long long);
vector unsigned long long vec_cnttz (vector unsigned long long);
gcc/testsuite/ChangeLog:
2017-07-14 Carl Love
* gcc.target/powerpc/builtins-4-9-runable.c: Add test file for
vec_cnttz builtins.
---
gcc/doc/extend.texi| 9 ++
.../gcc.target/powerpc/builtins-4-p9-runnable.c| 95 ++
2 files changed, 104 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/powerpc/builtins-4-p9-runnable.c
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 0135fc7..8df5498 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -15384,6 +15384,15 @@ vector bool short vec_cmpnez (vector unsigned short
arg1, vector unsigned short
vector bool int vec_cmpnez (vector signed int arg1, vector signed int arg2);
vector bool int vec_cmpnez (vector unsigned int, vector unsigned int);
+vector signed char vec_cnttz (vector signed char);
+vector unsigned char vec_cnttz (vector unsigned char);
+vector signed short vec_cnttz (vector signed short);
+vector unsigned short vec_cnttz (vector unsigned short);
+vector signed int vec_cnttz (vector signed int);
+vector unsigned int vec_cnttz (vector unsigned int);
+vector signed long long vec_cnttz (vector signed long long);
+vector unsigned long long vec_cnttz (vector unsigned long long);
+
signed int vec_cntlz_lsbb (vector signed char);
signed int vec_cntlz_lsbb (vector unsigned char);
diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-4-p9-runnable.c
b/gcc/testsuite/gcc.target/powerpc/builtins-4-p9-runnable.c
new file mode 100644
index 000..79fbaa8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/builtins-4-p9-runnable.c
@@ -0,0 +1,95 @@
+/* { dg-do run { target { powerpc64*-*-* && { lp64 && p9vector_hw } } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power9" } } */
+/* { dg-options "-mcpu=power9 -O2 -mupper-regs-di" } */
+
+#include // vector
+
+void abort (void);
+
+int main() {
+ int i;
+
+ vector signed char vsca, vscr, vscexpt;
+ vector unsigned char vuca, vucr, vucexpt;
+ vector signed short int vssa, vssr, vssexpt;
+ vector unsigned short int vusa, vusr, vusexpt;
+ vector signed int vsia, vsir, vsiexpt;
+ vector unsigned int vuia, vuir, vuiexpt;
+ vector signed long long vslla, vsllr, vsllexpt;
+ vector unsigned long long vulla, vullr, vullexpt;
+
+ vsca = (vector signed char) {0, 1, 2, 3, 4, 5, 6, 7,
+ 8, 9, 10, 11, 12, 13, 14, 15};
+
+ vscexpt = (vector signed char) {8, 0, 1, 0, 2, 0, 1, 0,
+ 3, 0, 1, 0, 2, 0, 1, 0};
+
+ vuca = (vector unsigned char) {'0', '3', '6', '9', 'A', 'B', 'E', 'F',
+'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N'};
+
+ vucexpt = (vector unsigned char) {4, 0, 1, 0, 0, 1, 0, 1,
+ 0, 3, 0, 1, 0, 2, 0, 1};
+
+ vssa = (vector short int) {0x1, 0x10, 0x100, 0x1000,
+0x2, 0x20, 0x200, 0x2000};
+
+ vssexpt = (vector short int) {0, 4, 8, 12, 1, 5, 9, 13};
+
+ vusa = (vector unsigned short int) {0x4, 0x40, 0x400, 0x4000,
+ 0x8, 0x80, 0x800, 0x8000};
+ vusexpt = (vector unsigned short int) {2, 6, 10, 14, 3, 7, 11, 15};
+
+ vsia = (vector int) {0x1, 0x10, 0x100, 0x1000};
+ vsiexpt = (vector int){16, 20, 24, 28};
+
+ vuia = (vector unsigned int) {0x2, 0x20, 0x200, 0x2000};
+ vuiexpt = (vector unsigned int){1, 5, 9, 13};
+
+ vslla = (vector long long) {0x0001LL, 0x000100010001LL};
+ vsllexpt = (vector long long){16, 16};
+
+ vulla = (vector unsigned long long) {0x4000LL,
0x0080ULL};
+
+ vullexpt = (vector unsigned long long) {46, 55};
+
+ vscr = vec_cnttz (vsca);
+ vucr = vec_cnttz (vuca);
+ vssr = vec_cnttz (vssa);
+ vusr = vec_cnttz (vusa);
+ vsir = vec_cnttz (vsia);
+ vuir = vec_cnttz (vuia);
+ vsllr = vec_cnttz (vslla);
+ vullr = vec_cnttz (vulla);
+
+ for (i=0; i<16; i++) {
+if (vscr[i] != vscexpt[i])
+ abort();
+
+if (vucr[i] != vucexpt[i])
+ abort();
+ }
+
+ for (i=0; i<8; i++) {
+if (vssr[i] != vssexpt[i])
+
Re: C PATCH to display types when printing a conversion warning (PR c/81233)
On 07/14/2017 09:40 AM, Martin Sebor wrote: On 07/14/2017 07:47 AM, Marek Polacek wrote: On Fri, Jul 14, 2017 at 02:52:36PM +0200, Marek Polacek wrote: On Thu, Jul 13, 2017 at 11:42:15AM -0600, Martin Sebor wrote: On 07/13/2017 08:18 AM, Marek Polacek wrote: This patch improves diagnostic in the C FE by printing the types when reporting a problem with a conversion. E.g., instead of warning: assignment from incompatible pointer type you'll now get warning: assignment to 'int *' from incompatible pointer type 'char *' or instead of warning: initialization makes integer from pointer without a cast this warning: initialization of 'int *' from 'int' makes pointer from integer without a cast I've been wanting this for a long time and here it is. Two snags: I had to make pedwarn_init to take '...' for which I had to introduce emit_diagnostic_valist; you can't pass varargs from one vararg function to another vararg function (and a macro with __VA_ARGS__ didn't work here). Also, PEDWARN_FOR_ASSIGNMENT didn't work with the addition of printing TYPE and RHSTYPE so I just decided to unroll the macro instead of making it even more ugly. This patch is long but it's mainly because of the testsuite fallout. If you have better ideas about the wording, let me know. It looks pretty good as is. My only wording suggestion is to consider simply mentioning conversion in the text of the warnings: warning: conversion to T* from an incompatible type U* I'm not sure that being explicit about the context where the bad conversion takes place (initialization vs assignment vs returning a value) is terribly helpful. That would not only simplify the code and make all the messages consistent, but it would also make it possible to get rid of the note when passing arguments. Yeah, I agree, actually. We print the expressions in question (although, we could probably do even better), and I don't see why it would be necessary to mention whether it's an initialization or an assignment. I think I'll just drop this patch and do something along the lines you suggest. David, do you agree with this? (Joseph's on PTO but I'd of course like to hear his opinion, too.) I think I changed my mind. Because clang says e.g. warning: returning 'unsigned int *' from a function with result type 'int *' converts between pointers to integer types with different sign so in the end it might be best to just go with my current patch; it should only improve things anyway. And then add the fix-it hint. Yes, Clang does do that. G++, OTOH, prints an error with the same text in all these cases. It's not tremendously important which of the two forms is used. What I do think would be nice is if the text of the same diagnostics, whether warnings or errors, could be more consistent between the front ends. A good way to do that in general, if all of the checking code cannot be shared, is to provide a shared diagnose_this_or_that() function for each diagnostic. The function would be parameterized on the kind of diagnostic (i.e., error or warning), but would hardcode the shared text. Each FE would call it with an argument telling it whether to issue it as an error or warning. Just to be clear: I don't mean to suggest to do this in this patch or necessarily even for this warning. I'm not even sure to what extent it might be doable. I mention it mostly as food for thought. Martin One more thing: for int *q = p; int i = q; we should be able to provide a fix-it hint, something like 'did you mean to dereference q?'. With the current PEDWARN_FOR_ASSIGNMENT macro it would be awkward to implement that. There are still more warnings to improve but I think better to do this incrementally rather than a single humongous patch. That makes sense. I was going to mention that it would be nice to also improve: warning: comparison of distinct pointer types lacks a cast If you take the conversion suggestion I think this warning would need to be phrased in terms of "conversion between T* and U*" rather than "conversion from T* to U*". (A similar change could be made to the error message printed when incompatible pointers are subtracted from one another.) Sure. Marek Marek
Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
On Fri, Jul 14, 2017 at 1:35 AM, Martin Liška wrote: > May I ask Jim to test the patch? > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. I started an aarch64 bootstrap to test. My fast machine is busy with work tasks, so I have to use a slower machine, and hence this will take some time. Jim
Re: [PATCH, rs6000 V2] Add support for vec_extract_fp_from_shorth() and vec_extract_fp_from_short
Hi Carl, > Also, per the note from Segher I could use xxperm instead of vperm, I > stayed with the vperm instruction. I don't see any functional or > performance advantage of the xxperm over vperm. xxperm does the same as vperm, but for all vector registers instead of only v0..v31. Since xvcvhpsp allows all registers as well, there is no real reason to restrict to "v" (instead of "wa") as far as I see. > +;; Generate vector extract four float 32 values from left four elements > +;; of eight element vector of float 16 values. > +(define_expand "vextract_fp_from_shorth" > + [(set (match_operand:V4SF 0 "register_operand" "=v") > + (unspec:V4SF [(match_operand:V8HI 1 "register_operand" "v")] > + UNSPEC_VSX_VEXTRACT_FP_FROM_SHORTH))] Nit: this last line should be indented the same as the [ on the line above (both are arguments to the unspec). Okay for trunk with those fixed. Thanks, Segher
[PATCH] Constrain std::variant constructor for class template argument deduction
This patch from Jason solves the problem pointed out at the end of
D0702R1 (not public yet):
std::variant v1(3);
std::variant v2 = v1;
Tested powerpc64le-linux, committed to trunk.
commit 54e520fc40bff5fe441715f23b64c2c788db23dc
Author: Jonathan Wakely
Date: Fri Jul 14 19:38:51 2017 +0100
Constrain std::variant constructor for class template argument deduction
2017-07-14 Jason Merrill
Jonathan Wakely
* include/std/variant (variant::variant(_Tp&&)): Constrain to remove
the constructor for empty variants from the candidate functions
during class template argument deduction.
* testsuite/20_util/variant/deduction.cc: New.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 06646f6..23db6d2 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1101,6 +1101,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template, variant>>,
+ typename = enable_if_t<(sizeof...(_Types)>0)>,
typename = enable_if_t<__exactly_once<__accepted_type<_Tp&&>>
&& is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>>>
constexpr
diff --git a/libstdc++-v3/testsuite/20_util/variant/deduction.cc
b/libstdc++-v3/testsuite/20_util/variant/deduction.cc
new file mode 100644
index 000..33bc9d2
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/deduction.cc
@@ -0,0 +1,39 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++1z } }
+
+// Copyright (C) 2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+//
Re: gotools patch committed: Test runtime, misc/cgo/{test,testcarchive}
On Thu, Jul 13, 2017 at 9:56 PM, Ian Lance Taylor wrote: > On Thu, Jun 29, 2017 at 11:40 PM, Uros Bizjak wrote: >> >>> This patch to the gotools Makefile adds tests to `make check`. We now >>> test the runtime package using the newly built go tool, and test that >>> cgo works by running the misc/cgo/test and misc/cgo/testcarchive >>> tests. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. >>> Committed to mainline. >> >> There are several failures on non-split targets, e.g.: >> >> FAIL: TestCgoHandlesWlORIGIN >> go_test.go:267: running testgo [build origin] >> go_test.go:286: standard error: >> go_test.go:287: # origin >> cc1: error: '-fsplit-stack' requires assembler support for CFI >> directives >> cc1: error: '-fsplit-stack' is not supported by this compiler >> configuration >> >> and: >> >> FAIL: TestCgoCrashHandler >> crash_test.go:70: building testprogcgo []: exit status 2 >> # _/home/uros/git/gcc/libgo/go/runtime/testdata/testprogcgo >> cc1: error: '-fsplit-stack' requires assembler support for CFI >> directives >> cc1: error: '-fsplit-stack' is not supported by this compiler >> configuration >> >> As evident from TestBuildDryRunWithCgo dump, -fsplit-stack argument is >> added unconditionally to the compile flags. > > Would you mind checking whether this patch fixes the problem on your > system? Thanks. Yes, this patch correctly detects that the compiler doesn't support -fsplit-stack. Thanks, Uros.
Re: [PATCH] Remove Java references in source code.
On 07/14/2017 11:43 AM, Rainer Orth wrote: > Hi Martin, > >> Hello. >> >> Patch removes some remaining references in gcc/ subfolder. >> >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > not enough, it seems: this patch broke Ada bootstrap at on > x86_64-pc-linux-gnu, sparc-sun-solaris2.12, and i386-pc-solaris2.12 > (probably everywhere) during stage2: > > +===GNAT BUG DETECTED==+ > | 8.0.0 20170714 (experimental) [trunk revision 250207] (x86_64-pc-linux-gnu) > GCC error:| > | in gimplify_save_expr, at gimplify.c:5805| > | Error detected around /vol/gcc/src/hg/trunk/local/gcc/ada/butil.adb:40:31| > | Please submit a bug report; see https://gcc.gnu.org/bugs/ . | > | Use a subject line meaningful to you and us to track the bug.| > | Include the entire contents of this bug box in the report. | > | Include the exact command that you entered. | > | Also include sources listed below. | > +==+ > > Please include these source files with error report > Note that list may not be accurate in some cases, > so please double check that the problem can still > be reproduced with the set of files listed. > Consider also -gnatd.n switch (see debug.adb). > > /vol/gcc/src/hg/trunk/local/gcc/ada/system.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/butil.adb > /vol/gcc/src/hg/trunk/local/gcc/ada/butil.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/namet.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/alloc.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/hostparm.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/types.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/unchconv.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/unchdeal.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/table.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/gnat.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/g-table.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/g-dyntab.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/ada.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/a-unccon.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/output.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/s-os_lib.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/s-string.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/s-stalib.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/s-exctab.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/s-unstyp.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/s-conca2.ads > /vol/gcc/src/hg/trunk/local/gcc/ada/s-assert.ads > > > raised TYPES.UNRECOVERABLE_ERROR : comperr.adb:406 > /vol/gcc/src/hg/trunk/local/gcc/ada/gcc-interface/Make-lang.in:119: recipe > for target 'ada/butil.o' failed > make[3]: *** [ada/butil.o] Error 1 > > Reverting ... > >> * gimplify.c >> (gimplify_save_expr): Likewise. > > ... this part allowed the bootstrap to finish. If you wanted to commit that change with a comment somehow indicating how the code is interacting with the Ada front-end, consider it pre-approved. Similarly for you Martin. No need to get approval for putting a chunk of code back in :-) jeff
[PATCH, committed] Fix PR81162
Hi,
PR81162 identifies a bug in SLSR involving overflow that occurs when
replacing a NEGATE_EXPR with a PLUS_EXPR. This is another example
of an unprofitable transformation that should be skipped anyway,
hence this simple patch. Bootstrapped and tested on
powerpc64le-unknown-linux-gnu, committed. Test case provided from
the bug report.
Thanks,
Bill
[gcc]
2016-07-14 Bill Schmidt
PR tree-optimization/81162
* gimple-ssa-strength-reduction.c (replace_mult_candidate): Don't
replace a negate with an add.
[gcc/testsuite]
2016-07-14 Bill Schmidt
PR tree-optimization/81162
* gcc.dg/pr81162.c: New file.
Index: gcc/gimple-ssa-strength-reduction.c
===
--- gcc/gimple-ssa-strength-reduction.c (revision 250189)
+++ gcc/gimple-ssa-strength-reduction.c (working copy)
@@ -2082,13 +2082,14 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
types but allows for safe negation without twisted logic. */
if (wi::fits_shwi_p (bump)
&& bump.to_shwi () != HOST_WIDE_INT_MIN
- /* It is not useful to replace casts, copies, or adds of
+ /* It is not useful to replace casts, copies, negates, or adds of
an SSA name and a constant. */
&& cand_code != SSA_NAME
&& !CONVERT_EXPR_CODE_P (cand_code)
&& cand_code != PLUS_EXPR
&& cand_code != POINTER_PLUS_EXPR
- && cand_code != MINUS_EXPR)
+ && cand_code != MINUS_EXPR
+ && cand_code != NEGATE_EXPR)
{
enum tree_code code = PLUS_EXPR;
tree bump_tree;
Index: gcc/testsuite/gcc.dg/pr81162.c
===
--- gcc/testsuite/gcc.dg/pr81162.c (nonexistent)
+++ gcc/testsuite/gcc.dg/pr81162.c (working copy)
@@ -0,0 +1,17 @@
+/* PR tree-optimization/81162 */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=undefined -O2" } */
+
+short s;
+int i1 = 1;
+int i2 = 1;
+unsigned char uc = 147;
+
+int main() {
+ s = (-uc + 2147483647) << 0;
+ if (9031239389974324562ULL >= (-((i1 && i2) + uc) ^ -21096) ) {
+return 0;
+ } else {
+return -1;
+ }
+}
Re: [C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")
On 07/14/2017 01:32 PM, Paolo Carlini wrote: While working on the bug I also noticed that we can simplify a bit the code generating the implicit deduction guides: if I'm not mistaken, when we pass types as first argument of build_deduction_guide - for implicit guides, that is - the deduction guide is never explicit. thus DECL_NONCONVERTING_P is never true. It's an unrelated tweak, anyway, which we can consider applying by itself if we don't change the code generating the implicit deduction guides. I recall wondering the same thing when adding the 'elided = true' pieces, but didn't investigate enough to confirm/deny. Thanks for getting this. nathan -- Nathan Sidwell
Re: [PATCH] Remove Java references in source code.
Hi Martin, > Hello. > > Patch removes some remaining references in gcc/ subfolder. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. not enough, it seems: this patch broke Ada bootstrap at on x86_64-pc-linux-gnu, sparc-sun-solaris2.12, and i386-pc-solaris2.12 (probably everywhere) during stage2: +===GNAT BUG DETECTED==+ | 8.0.0 20170714 (experimental) [trunk revision 250207] (x86_64-pc-linux-gnu) GCC error:| | in gimplify_save_expr, at gimplify.c:5805| | Error detected around /vol/gcc/src/hg/trunk/local/gcc/ada/butil.adb:40:31| | Please submit a bug report; see https://gcc.gnu.org/bugs/ . | | Use a subject line meaningful to you and us to track the bug.| | Include the entire contents of this bug box in the report. | | Include the exact command that you entered. | | Also include sources listed below. | +==+ Please include these source files with error report Note that list may not be accurate in some cases, so please double check that the problem can still be reproduced with the set of files listed. Consider also -gnatd.n switch (see debug.adb). /vol/gcc/src/hg/trunk/local/gcc/ada/system.ads /vol/gcc/src/hg/trunk/local/gcc/ada/butil.adb /vol/gcc/src/hg/trunk/local/gcc/ada/butil.ads /vol/gcc/src/hg/trunk/local/gcc/ada/namet.ads /vol/gcc/src/hg/trunk/local/gcc/ada/alloc.ads /vol/gcc/src/hg/trunk/local/gcc/ada/hostparm.ads /vol/gcc/src/hg/trunk/local/gcc/ada/types.ads /vol/gcc/src/hg/trunk/local/gcc/ada/unchconv.ads /vol/gcc/src/hg/trunk/local/gcc/ada/unchdeal.ads /vol/gcc/src/hg/trunk/local/gcc/ada/table.ads /vol/gcc/src/hg/trunk/local/gcc/ada/gnat.ads /vol/gcc/src/hg/trunk/local/gcc/ada/g-table.ads /vol/gcc/src/hg/trunk/local/gcc/ada/g-dyntab.ads /vol/gcc/src/hg/trunk/local/gcc/ada/ada.ads /vol/gcc/src/hg/trunk/local/gcc/ada/a-unccon.ads /vol/gcc/src/hg/trunk/local/gcc/ada/output.ads /vol/gcc/src/hg/trunk/local/gcc/ada/s-os_lib.ads /vol/gcc/src/hg/trunk/local/gcc/ada/s-string.ads /vol/gcc/src/hg/trunk/local/gcc/ada/s-stalib.ads /vol/gcc/src/hg/trunk/local/gcc/ada/s-exctab.ads /vol/gcc/src/hg/trunk/local/gcc/ada/s-unstyp.ads /vol/gcc/src/hg/trunk/local/gcc/ada/s-conca2.ads /vol/gcc/src/hg/trunk/local/gcc/ada/s-assert.ads raised TYPES.UNRECOVERABLE_ERROR : comperr.adb:406 /vol/gcc/src/hg/trunk/local/gcc/ada/gcc-interface/Make-lang.in:119: recipe for target 'ada/butil.o' failed make[3]: *** [ada/butil.o] Error 1 Reverting ... > * gimplify.c > (gimplify_save_expr): Likewise. ... this part allowed the bootstrap to finish. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
[PATCH, rs6000 V2] Add support for vec_extract_fp_from_shorth() and vec_extract_fp_from_short
GCC Maintainers:
The following patch adds support for the vec_extract_fp_from_shorth()
and vec_extract_fp_from_short builtin functions has been updated per
Segher's comments. The patch has been re-tested on
powerpc64le-unknown-linux-gnu (Power 8 LE) and
powerpc64le-unknown-linux-gnu (Power 9 LE). The test generates 1
unsupported test on Power 8 and 2 test passes on Power 9.
Note, I verified that "dg-require-effective-target powerpc_p9vector_ok"
is not required in the test case.
Also, per the note from Segher I could use xxperm instead of vperm, I
stayed with the vperm instruction. I don't see any functional or
performance advantage of the xxperm over vperm.
Please let me know if the following patch is acceptable. Thanks.
Carl Love
gcc/ChangeLog:
2017-07-14 Carl Love
* config/rs6000/rs6000-c.c: Add support for built-in functions
vector float vec_extract_fp32_from_shorth (vector unsigned short);
vector float vec_extract_fp32_from_shortl (vector unsigned short);
* config/rs6000/altivec.h (vec_extract_fp_from_shorth,
vec_extract_fp_from_shortl): Add defines for the two builtins.
* config/rs6000/rs6000-builtin.def (VEXTRACT_FP_FROM_SHORTH,
VEXTRACT_FP_FROM_SHORTL): Add BU_P9V_OVERLOAD_1 and BU_P9V_VSX_1
new builtins.
* config/rs6000/vsx.md vsx_xvcvhpsp): Add define_insn.
(vextract_fp_from_shorth, vextract_fp_from_shortl): Add define_expands.
* doc/extend.texi: Update the built-in documentation file for the
new built-in function.
gcc/testsuite/ChangeLog:
2017-07-14 Carl Love
* gcc.target/powerpc/builtins-3-p9-runnable.c: Add new test file for
the new built-ins.
---
gcc/config/rs6000/altivec.h| 3 +
gcc/config/rs6000/rs6000-builtin.def | 5 ++
gcc/config/rs6000/rs6000-c.c | 5 ++
gcc/config/rs6000/vsx.md | 70 +-
gcc/doc/extend.texi| 3 +
.../gcc.target/powerpc/builtins-3-p9-runnable.c| 35 +++
6 files changed, 120 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/powerpc/builtins-3-p9-runnable.c
diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h
index 71cdca5..4d34a97 100644
--- a/gcc/config/rs6000/altivec.h
+++ b/gcc/config/rs6000/altivec.h
@@ -449,6 +449,9 @@
#define vec_insert_exp __builtin_vec_insert_exp
#define vec_test_data_class __builtin_vec_test_data_class
+#define vec_extract_fp_from_shorth __builtin_vec_vextract_fp_from_shorth
+#define vec_extract_fp_from_shortl __builtin_vec_vextract_fp_from_shortl
+
#define scalar_extract_exp __builtin_vec_scalar_extract_exp
#define scalar_extract_sig __builtin_vec_scalar_extract_sig
#define scalar_insert_exp __builtin_vec_scalar_insert_exp
diff --git a/gcc/config/rs6000/rs6000-builtin.def
b/gcc/config/rs6000/rs6000-builtin.def
index e098e1c..400189e 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -2057,6 +2057,9 @@ BU_P9V_OVERLOAD_1 (VSTDCNSP, "scalar_test_neg_sp")
BU_P9V_OVERLOAD_1 (REVB, "revb")
+BU_P9V_OVERLOAD_1 (VEXTRACT_FP_FROM_SHORTH, "vextract_fp_from_shorth")
+BU_P9V_OVERLOAD_1 (VEXTRACT_FP_FROM_SHORTL, "vextract_fp_from_shortl")
+
/* ISA 3.0 vector scalar overloaded 2 argument functions. */
BU_P9V_OVERLOAD_2 (VSIEDP, "scalar_insert_exp")
@@ -2074,6 +2077,8 @@ BU_P9V_VSX_1 (VEEDP, "extract_exp_dp", CONST, xvxexpdp)
BU_P9V_VSX_1 (VEESP, "extract_exp_sp", CONST, xvxexpsp)
BU_P9V_VSX_1 (VESDP, "extract_sig_dp", CONST, xvxsigdp)
BU_P9V_VSX_1 (VESSP, "extract_sig_sp", CONST, xvxsigsp)
+BU_P9V_VSX_1 (VEXTRACT_FP_FROM_SHORTH, "vextract_fp_from_shorth", CONST,
vextract_fp_from_shorth)
+BU_P9V_VSX_1 (VEXTRACT_FP_FROM_SHORTL, "vextract_fp_from_shortl", CONST,
vextract_fp_from_shortl)
/* 2 argument vsx vector functions added in ISA 3.0 (power9). */
BU_P9V_VSX_2 (VIEDP, "insert_exp_dp", CONST, xviexpdp)
diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c
index c769442..a1d09ba 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -5164,6 +5164,11 @@ const struct altivec_builtin_types
altivec_overloaded_builtins[] = {
{ P9V_BUILTIN_VEC_VEXTRACT4B, P9V_BUILTIN_VEXTRACT4B,
RS6000_BTI_INTDI, RS6000_BTI_unsigned_V16QI, RS6000_BTI_UINTSI, 0 },
+ { P9V_BUILTIN_VEC_VEXTRACT_FP_FROM_SHORTH,
P9V_BUILTIN_VEXTRACT_FP_FROM_SHORTH,
+RS6000_BTI_V4SF, RS6000_BTI_unsigned_V8HI, 0, 0 },
+ { P9V_BUILTIN_VEC_VEXTRACT_FP_FROM_SHORTL,
P9V_BUILTIN_VEXTRACT_FP_FROM_SHORTL,
+RS6000_BTI_V4SF, RS6000_BTI_unsigned_V8HI, 0, 0 },
+
{ P9V_BUILTIN_VEC_VEXTULX, P9V_BUILTIN_VEXTUBLX,
RS6000_BTI_INTQI, RS6000_BTI_UINTSI,
RS6000_BTI_V16QI, 0 },
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index 2ddfae5..7fac5f9
[C++ Patch] PR 79790 ("[C++17] ICE class template argument deduction")
Hi,
in this C++17 ICE on invalid we crash when, in do_class_deduction,
build_new_function_call is called with a null first argument. What is
happening is pretty simple to analyze: for the broken snippet, there are
no cands and elided is false, because the issue isn't that candidates
are elided because explicit, instead that there are no viable candidates
whatsoever, because args->length () == 3 and we don't even have the
implicit deduction guides. Alternately to the straightforward fix I'm
proposing below, a literal reading of the standard [16.3.1.8] suggests
that we could also consider generating the implicit deduction guides
even when args->length () >= 2 and the copy deduction guide even when
args->length () != 1, that is unconditionally.
While working on the bug I also noticed that we can simplify a bit the
code generating the implicit deduction guides: if I'm not mistaken, when
we pass types as first argument of build_deduction_guide - for implicit
guides, that is - the deduction guide is never explicit. thus
DECL_NONCONVERTING_P is never true. It's an unrelated tweak, anyway,
which we can consider applying by itself if we don't change the code
generating the implicit deduction guides.
Thanks! Paolo.
///
/cp
2017-07-14 Paolo Carlini
PR c++/79790
* pt.c (do_class_deduction): Handle the case of no viable implicit
deduction guides; simplify the code generating implicit deduction
guides.
/testsuite
2017-07-14 Paolo Carlini
PR c++/79790
* g++.dg/cpp1z/class-deduction42.C: New.
Index: cp/pt.c
===
--- cp/pt.c (revision 250186)
+++ cp/pt.c (working copy)
@@ -25439,11 +25439,7 @@ do_class_deduction (tree ptype, tree tmpl, tree in
if (gtype)
{
tree guide = build_deduction_guide (gtype, outer_args, complain);
- if ((flags & LOOKUP_ONLYCONVERTING)
- && DECL_NONCONVERTING_P (STRIP_TEMPLATE (guide)))
- elided = true;
- else
- cands = lookup_add (guide, cands);
+ cands = lookup_add (guide, cands);
}
}
@@ -25454,6 +25450,12 @@ do_class_deduction (tree ptype, tree tmpl, tree in
"user-declared constructors", type);
return error_mark_node;
}
+ else if (!cands && call == error_mark_node)
+{
+ error ("cannot deduce template arguments for copy-initialization"
+" of %qT, as it has no viable implicit deduction guides", type);
+ return error_mark_node;
+}
if (call == error_mark_node)
{
Index: testsuite/g++.dg/cpp1z/class-deduction42.C
===
--- testsuite/g++.dg/cpp1z/class-deduction42.C (revision 0)
+++ testsuite/g++.dg/cpp1z/class-deduction42.C (working copy)
@@ -0,0 +1,10 @@
+// PR c++/79790
+// { dg-options -std=c++1z }
+
+template
+struct array
+{
+ int a [N];
+};
+
+array a = { 1, 2, 3 }; // { dg-error "cannot deduce" }
Re: [PATCH] Kill TYPE_METHODS ipa 2/9
> This patch changes ipa-devirt. It contains a loop checking all the member
> functions are 'the same'. However, as the comment says, we've already
> zapped TYPE_METHODS, so the loop is never entered. I've added equivalent
> zapping in this patch series, as the rationale appears to be reducing memory
> footprint. It didn't seem appropriate to merge into the TYPE_FIELDS loop,
> given it's never exercised.
>
> I'd appreciate review.
>
> nathan
>
> --
> Nathan Sidwell
> 2017-07-14 Nathan Sidwell
>
> gcc/
> * ipa-devirt.c (odr_types_equivalent_p): Delete TYPE_METHODS scan.
OK (we get virtual methods checked for match by comparing virtual tables,
methods of same name are ODR checked, so we only miss diagnostics when
one unit defines non-virtual method while other doesn't. It would be nice
to check this, too, but there are cheaper ways of doing this)
Honza
>
> Index: gcc/ipa-devirt.c
> ===
> --- gcc/ipa-devirt.c (revision 250160)
> +++ gcc/ipa-devirt.c (working copy)
> @@ -1602,62 +1602,6 @@ odr_types_equivalent_p (tree t1, tree t2
>
> return false;
> }
> - if ((TYPE_MAIN_VARIANT (t1) == t1 || TYPE_MAIN_VARIANT (t2) == t2)
> - && COMPLETE_TYPE_P (TYPE_MAIN_VARIANT (t1))
> - && COMPLETE_TYPE_P (TYPE_MAIN_VARIANT (t2))
> - && odr_type_p (TYPE_MAIN_VARIANT (t1))
> - && odr_type_p (TYPE_MAIN_VARIANT (t2))
> - && (TYPE_METHODS (TYPE_MAIN_VARIANT (t1))
> - != TYPE_METHODS (TYPE_MAIN_VARIANT (t2
> - {
> - /* Currently free_lang_data sets TYPE_METHODS to error_mark_node
> -if it is non-NULL so this loop will never realy execute. */
> - if (TYPE_METHODS (TYPE_MAIN_VARIANT (t1)) != error_mark_node
> - && TYPE_METHODS (TYPE_MAIN_VARIANT (t2)) != error_mark_node)
> - for (f1 = TYPE_METHODS (TYPE_MAIN_VARIANT (t1)),
> -f2 = TYPE_METHODS (TYPE_MAIN_VARIANT (t2));
> -f1 && f2 ; f1 = DECL_CHAIN (f1), f2 = DECL_CHAIN (f2))
> - {
> - if (DECL_ASSEMBLER_NAME (f1) != DECL_ASSEMBLER_NAME (f2))
> - {
> - warn_odr (t1, t2, f1, f2, warn, warned,
> - G_("a different method of same type "
> -"is defined in another "
> -"translation unit"));
> - return false;
> - }
> - if (DECL_VIRTUAL_P (f1) != DECL_VIRTUAL_P (f2))
> - {
> - warn_odr (t1, t2, f1, f2, warn, warned,
> - G_("a definition that differs by virtual "
> -"keyword in another translation unit"));
> - return false;
> - }
> - if (DECL_VINDEX (f1) != DECL_VINDEX (f2))
> - {
> - warn_odr (t1, t2, f1, f2, warn, warned,
> - G_("virtual table layout differs "
> -"in another translation unit"));
> - return false;
> - }
> - if (odr_subtypes_equivalent_p (TREE_TYPE (f1),
> - TREE_TYPE (f2), visited,
> - loc1, loc2))
> - {
> - warn_odr (t1, t2, f1, f2, warn, warned,
> - G_("method with incompatible type is "
> -"defined in another translation unit"));
> - return false;
> - }
> - }
> - if ((f1 == NULL) != (f2 == NULL))
> - {
> - warn_odr (t1, t2, NULL, NULL, warn, warned,
> - G_("a type with different number of methods "
> - "is defined in another translation unit"));
> - return false;
> - }
> - }
> }
> break;
>}
Re: [PATCH] Kill TYPE_METHODS 9/9
And finally, this patch kills the TYPE_METHODS macro. As I mentioned earlier, I have other patches to repurpose the now-unused slot in records and unions. nathan -- Nathan Sidwell 2017-07-14 Nathan Sidwell gcc/ * tree.h (TYPE_METHODS): Delete. Index: gcc/tree.h === --- gcc/tree.h (revision 250160) +++ gcc/tree.h (working copy) @@ -2110,8 +2110,6 @@ extern machine_mode element_mode (const_ (FUNC_OR_METHOD_CHECK (NODE)->type_non_common.values) #define TYPE_VALUES_RAW(NODE) (TYPE_CHECK (NODE)->type_non_common.values) -#define TYPE_METHODS(NODE) \ - (RECORD_OR_UNION_CHECK (NODE)->type_non_common.maxval) #define TYPE_VFIELD(NODE) \ (RECORD_OR_UNION_CHECK (NODE)->type_non_common.minval) #define TYPE_METHOD_BASETYPE(NODE) \
Re: [PATCH] Kill TYPE_METHODS objc 8/9
This bit of the objective C runtime already knows that TYPE_FIELDS might contain
non-FIELD_DECL things. But it has the delightful assumption that the first
thing is a FIELD_DECL, which apparently was fine until now. Fixed by
simplifying the loop.
nathan
--
Nathan Sidwell
2017-07-14 Nathan Sidwell
gcc/objc/
* objc-runtime-shared-support.c (build_ivar_list_initializer):
Don't presume first item is a FIELD_DECL.
Index: gcc/objc/objc-runtime-shared-support.c
===
--- gcc/objc/objc-runtime-shared-support.c (revision 250160)
+++ gcc/objc/objc-runtime-shared-support.c (working copy)
@@ -528,34 +528,32 @@ build_ivar_list_initializer (tree type,
{
vec *inits = NULL;
- do
-{
- vec *ivar = NULL;
- tree id;
+ for (; field_decl; field_decl = DECL_CHAIN (field_decl))
+if (TREE_CODE (field_decl) == FIELD_DECL)
+ {
+ vec *ivar = NULL;
+ tree id;
- /* Set name. */
- if (DECL_NAME (field_decl))
- CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE,
- add_objc_string (DECL_NAME (field_decl),
-meth_var_names));
- else
- /* Unnamed bit-field ivar (yuck). */
- CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE, build_int_cst (NULL_TREE, 0));
+ /* Set name. */
+ if (DECL_NAME (field_decl))
+ CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE,
+ add_objc_string (DECL_NAME (field_decl),
+ meth_var_names));
+ else
+ /* Unnamed bit-field ivar (yuck). */
+ CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE,
+ build_int_cst (NULL_TREE, 0));
- /* Set type. */
- id = add_objc_string (encode_field_decl (field_decl),
-meth_var_types);
- CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE, id);
+ /* Set type. */
+ id = add_objc_string (encode_field_decl (field_decl),
+ meth_var_types);
+ CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE, id);
- /* Set offset. */
- CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE, byte_position (field_decl));
- CONSTRUCTOR_APPEND_ELT (inits, NULL_TREE,
- objc_build_constructor (type, ivar));
- do
- field_decl = DECL_CHAIN (field_decl);
- while (field_decl && TREE_CODE (field_decl) != FIELD_DECL);
+ /* Set offset. */
+ CONSTRUCTOR_APPEND_ELT (ivar, NULL_TREE, byte_position (field_decl));
+ CONSTRUCTOR_APPEND_ELT (inits, NULL_TREE,
+ objc_build_constructor (type, ivar));
}
- while (field_decl);
return objc_build_constructor (build_array_type (type, 0), inits);
}
Re: [PATCH] Kill TYPE_METHODS misc 7/9
This bit removes knowledge of TYPE_METHODS from the various tree dumpers,
allocators and deleters.
As mentioned before, the only reason we set TYPE_METHODS to error_mark_node when
handing over to the middle end is for the RTL check about register allocation.
But I'm deleting that check.
nathan
--
Nathan Sidwell
2017-07-14 Nathan Sidwell
gcc/
* tree.c (free_lang_data_in_type): Stitch out member functions and
templates from TYPE_FIELDS.
(build_distinct_type_copy, verify_type_variant,
verify_type): Member fns are on TYPE_FIELDS.
* tree-dump.c (dequeue_and_dump): No TYPE_METHODS.
* tree-pretty-print.c (dump_generic_node): Likewise.
Index: gcc/tree-dump.c
===
--- gcc/tree-dump.c (revision 250160)
+++ gcc/tree-dump.c (working copy)
@@ -490,7 +490,6 @@ dequeue_and_dump (dump_info_p di)
dump_string_field (di, "tag", "union");
dump_child ("flds", TYPE_FIELDS (t));
- dump_child ("fncs", TYPE_METHODS (t));
queue_and_dump_index (di, "binf", TYPE_BINFO (t),
DUMP_BINFO);
break;
Index: gcc/tree-pretty-print.c
===
--- gcc/tree-pretty-print.c (revision 250160)
+++ gcc/tree-pretty-print.c (working copy)
@@ -1860,22 +1860,9 @@ dump_generic_node (pretty_printer *pp, t
dump_decl_name (pp, node, flags);
else if (TYPE_NAME (TREE_TYPE (node)) != node)
{
- if ((TREE_CODE (TREE_TYPE (node)) == RECORD_TYPE
- || TREE_CODE (TREE_TYPE (node)) == UNION_TYPE)
- && TYPE_METHODS (TREE_TYPE (node)))
- {
- /* The type is a c++ class: all structures have at least
-4 methods. */
- pp_string (pp, "class ");
- dump_generic_node (pp, TREE_TYPE (node), spc, flags, false);
- }
- else
- {
- pp_string (pp,
-(TREE_CODE (TREE_TYPE (node)) == UNION_TYPE
+ pp_string (pp, (TREE_CODE (TREE_TYPE (node)) == UNION_TYPE
? "union" : "struct "));
- dump_generic_node (pp, TREE_TYPE (node), spc, flags, false);
- }
+ dump_generic_node (pp, TREE_TYPE (node), spc, flags, false);
}
else
pp_string (pp, "");
Index: gcc/tree.c
===
--- gcc/tree.c (revision 250160)
+++ gcc/tree.c (working copy)
@@ -5221,13 +5221,15 @@ free_lang_data_in_type (tree type)
if (TYPE_VFIELD (type) && TREE_CODE (TYPE_VFIELD (type)) != FIELD_DECL)
TYPE_VFIELD (type) = NULL_TREE;
- /* Remove TYPE_METHODS list. While it would be nice to keep it
-to enable ODR warnings about different method lists, doing so
-seems to impractically increase size of LTO data streamed.
-Keep the information if TYPE_METHODS was non-NULL. This is used
-by function.c and pretty printers. */
- if (TYPE_METHODS (type))
-TYPE_METHODS (type) = error_mark_node;
+ /* Splice out FUNCTION_DECLS and TEMPLATE_DECLS from
+TYPE_FIELDS. So LTO doesn't grow. */
+ for (tree probe, *prev= &TYPE_FIELDS (type); (probe = *prev); )
+ if (TREE_CODE (probe) == FUNCTION_DECL
+ || TREE_CODE (probe) == TEMPLATE_DECL)
+ *prev = probe;
+ else
+ prev = &DECL_CHAIN (probe);
+
if (TYPE_BINFO (type))
{
free_lang_data_in_binfo (TYPE_BINFO (type));
@@ -5422,9 +5424,10 @@ free_lang_data_in_decl (tree decl)
At this point, it is not needed anymore. */
DECL_SAVED_TREE (decl) = NULL_TREE;
- /* Clear the abstract origin if it refers to a method. Otherwise
- dwarf2out.c will ICE as we clear TYPE_METHODS and thus the
-origin will not be output correctly. */
+ /* Clear the abstract origin if it refers to a method.
+ Otherwise dwarf2out.c will ICE as we splice functions out of
+ TYPE_FIELDS and thus the origin will not be output
+ correctly. */
if (DECL_ABSTRACT_ORIGIN (decl)
&& DECL_CONTEXT (DECL_ABSTRACT_ORIGIN (decl))
&& RECORD_OR_UNION_TYPE_P
@@ -6692,12 +6695,6 @@ build_distinct_type_copy (tree type MEM_
TYPE_MAIN_VARIANT (t) = t;
TYPE_NEXT_VARIANT (t) = 0;
- /* We do not record methods in type copies nor variants
- so we do not need to keep them up to date when new method
- is inserted. */
- if (RECORD_OR_UNION_TYPE_P (t))
-TYPE_METHODS (t) = NULL_TREE;
-
/* Note that it is now possible for TYPE_MIN_VALUE to be a value
whose TREE_TYPE is not t. This can also happen in the Ada
frontend when using subtypes. */
@@ -13423,8 +13420,6 @@ verify_type_variant (const_tree t, tree
- aggregates may have new TYPE_FIELDS list that list variants of
the main variant TYPE_F
Re: [PATCH] Kill TYPE_METHODS libcc1 6/9
This is the libcc1 change. When creating a clone, it needs to fiddle with
TYPE_FIELDS now.
nathan
--
Nathan Sidwell
2017-07-14 Nathan Sidwell
libcc1/
* libcp1plugin.cc (plugin_build_decl): Member fns are on TYPE_FIELDS.
Index: libcc1/libcp1plugin.cc
===
--- libcc1/libcp1plugin.cc (revision 250160)
+++ libcc1/libcp1plugin.cc (working copy)
@@ -1556,7 +1556,7 @@ plugin_build_decl (cc1_plugin::connectio
if ((ctor || dtor)
/* Don't crash after a duplicate declaration of a cdtor. */
- && TYPE_METHODS (current_class_type) == decl)
+ && TYPE_FIELDS (current_class_type) == decl)
{
/* ctors and dtors clones are chained after DECL.
However, we create the clones before TYPE_METHODS is
@@ -1568,9 +1568,9 @@ plugin_build_decl (cc1_plugin::connectio
tree save = DECL_CHAIN (decl);
DECL_CHAIN (decl) = NULL_TREE;
clone_function_decl (decl, /*update_methods=*/true);
- gcc_assert (TYPE_METHODS (current_class_type) == decl);
- TYPE_METHODS (current_class_type)
- = nreverse (TYPE_METHODS (current_class_type));
+ gcc_assert (TYPE_FIELDS (current_class_type) == decl);
+ TYPE_FIELDS (current_class_type)
+ = nreverse (TYPE_FIELDS (current_class_type));
DECL_CHAIN (decl) = save;
}
Re: [PATCH] Kill TYPE_METHODS c++ 5/9
This is the biggest chunk, in the C++ FE.
The only quirk is a hopefully short-lived additional check when looking up a
non-function member in an incomplete type. We iterate over TYPE_FIELDS, and
should now ignore things that are functions -- because we'll find them as
overloads when looking at the method-vector. That's a subsequent cleanup.
nathan
--
Nathan Sidwell
2017-07-14 Nathan Sidwell
gcc/cp/
* class.c (maybe_warn_about_overly_private_class,
finish_struct_methods, one_inheriting_sig, count_fields,
add_fields_to_record_type, check_field_decls, check_methods,
clone_function_decl, set_method_tm_attributes,
finalize_literal_type_property, check_bases_and_members,
create_vtable_ptr, determine_key_method,
unreverse_member_declarations, finish_struct,
add_vcall_offset_vtbl_entries_1): Member fns are on TYPE_FIELDS.
* decl.c (fixup_anonymous_aggr): Likewise.
* decl2.c (reset_type_linkage_2): Likewise.
* method.c (after_nsdmi_defaulted_late_checks,
lazily_declare_fn): Likewise.
* optimize.c (maybe_thunk_body, maybe_clone_body): Likewise.
* pt.c (instantiate_class_template_1, tsubst_expr,
do_type_instantiation, instantiate_pending_templates): Likewise.
* search.c (lookup_field_1): Likewise.
* semantics.c (finish_member_declaration,
finish_omp_declare_simd_methods): Likewise.
gcc/testsuite/
* g++.dg/ext/anon-struct6.C: Adjust diag.
* g++.old-deja/g++.other/anon4.C: Adjust diag.
Index: gcc/cp/class.c
===
--- gcc/cp/class.c (revision 250160)
+++ gcc/cp/class.c (working copy)
@@ -2149,7 +2149,6 @@ maybe_warn_about_overly_private_class (t
{
int has_member_fn = 0;
int has_nonprivate_method = 0;
- tree fn;
if (!warn_ctor_dtor_privacy
/* If the class has friends, those entities might create and
@@ -2179,26 +2178,26 @@ maybe_warn_about_overly_private_class (t
functions are private. (Since there are no friends or
non-private statics, we can't ever call any of the private member
functions.) */
- for (fn = TYPE_METHODS (t); fn; fn = DECL_CHAIN (fn))
-/* We're not interested in compiler-generated methods; they don't
- provide any way to call private members. */
-if (!DECL_ARTIFICIAL (fn))
+ for (tree fn = TYPE_FIELDS (t); fn; fn = DECL_CHAIN (fn))
+if (!DECL_DECLARES_FUNCTION_P (fn))
+ /* Not a function. */;
+else if (DECL_ARTIFICIAL (fn))
+ /* We're not interested in compiler-generated methods; they don't
+provide any way to call private members. */;
+else if (!TREE_PRIVATE (fn))
{
- if (!TREE_PRIVATE (fn))
- {
- if (DECL_STATIC_FUNCTION_P (fn))
- /* A non-private static member function is just like a
-friend; it can create and invoke private member
-functions, and be accessed without a class
-instance. */
- return;
+ if (DECL_STATIC_FUNCTION_P (fn))
+ /* A non-private static member function is just like a
+friend; it can create and invoke private member
+functions, and be accessed without a class
+instance. */
+ return;
- has_nonprivate_method = 1;
- /* Keep searching for a static member function. */
- }
- else if (!DECL_CONSTRUCTOR_P (fn) && !DECL_DESTRUCTOR_P (fn))
- has_member_fn = 1;
+ has_nonprivate_method = 1;
+ /* Keep searching for a static member function. */
}
+else if (!DECL_CONSTRUCTOR_P (fn) && !DECL_DESTRUCTOR_P (fn))
+ has_member_fn = 1;
if (!has_nonprivate_method && has_member_fn)
{
@@ -2228,14 +2227,14 @@ maybe_warn_about_overly_private_class (t
/* Even if some of the member functions are non-private, the class
won't be useful for much if all the constructors or destructors
are private: such an object can never be created or destroyed. */
- fn = CLASSTYPE_DESTRUCTOR (t);
- if (fn && TREE_PRIVATE (fn))
-{
- warning (OPT_Wctor_dtor_privacy,
- "%q#T only defines a private destructor and has no friends",
- t);
- return;
-}
+ if (tree dtor = CLASSTYPE_DESTRUCTOR (t))
+if (TREE_PRIVATE (dtor))
+ {
+ warning (OPT_Wctor_dtor_privacy,
+"%q#T only defines a private destructor and has no friends",
+t);
+ return;
+ }
/* Warn about classes that have private constructors and no friends. */
if (TYPE_HAS_USER_CONSTRUCTOR (t)
@@ -2367,7 +2366,6 @@ resort_type_method_vec (void* obj,
static void
finish_struct_methods (tree t)
{
- tree fn_fields;
vec *method_vec;
int slot, len;
@@ -2378,9 +2376,9 @@ finish_struct_methods (tree t)
len = method_vec->length ();
/* Clear DECL_IN_AGGR_P for a
Re: [PATCH] Kill TYPE_METHODS ada-spec 4/9
This change to the ada-spec generation teaches it about looking at TYPE_FIELDS.
quite straight forwards.
nathan
--
Nathan Sidwell
2017-07-14 Nathan Sidwell
gcc/c-family/
* c-ada-spec.c (is_tagged_type, has_nontrivial_methods,
dump_ada_template, print_ada_methods,
print_ada_declaration): Member fns are on TYPE_FIELDS.
Index: gcc/c-family/c-ada-spec.c
===
--- gcc/c-family/c-ada-spec.c (revision 250160)
+++ gcc/c-family/c-ada-spec.c (working copy)
@@ -1070,16 +1070,11 @@ has_static_fields (const_tree type)
static bool
is_tagged_type (const_tree type)
{
- tree tmp;
-
if (!type || !RECORD_OR_UNION_TYPE_P (type))
return false;
- /* TYPE_METHODS is only set on the main variant. */
- type = TYPE_MAIN_VARIANT (type);
-
- for (tmp = TYPE_METHODS (type); tmp; tmp = TREE_CHAIN (tmp))
-if (TREE_CODE (tmp) == FUNCTION_DECL && DECL_VINDEX (tmp))
+ for (tree fld = TYPE_FIELDS (type); fld; fld = TREE_CHAIN (fld))
+if (TREE_CODE (fld) == FUNCTION_DECL && DECL_VINDEX (fld))
return true;
return false;
@@ -1093,8 +1088,6 @@ is_tagged_type (const_tree type)
static bool
has_nontrivial_methods (tree type)
{
- tree tmp;
-
if (!type || !RECORD_OR_UNION_TYPE_P (type))
return false;
@@ -1106,12 +1099,9 @@ has_nontrivial_methods (tree type)
if (!cpp_check (type, IS_TRIVIAL))
return true;
- /* TYPE_METHODS is only set on the main variant. */
- type = TYPE_MAIN_VARIANT (type);
-
/* If there are user-defined methods, they are deemed non-trivial. */
- for (tmp = TYPE_METHODS (type); tmp; tmp = TREE_CHAIN (tmp))
-if (!DECL_ARTIFICIAL (tmp))
+ for (tree fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
+if (TREE_CODE (TREE_TYPE (fld)) == METHOD_TYPE && !DECL_ARTIFICIAL (fld))
return true;
return false;
@@ -1896,7 +1886,7 @@ dump_ada_template (pretty_printer *buffe
if (TREE_VEC_LENGTH (types) == 0)
break;
- if (!RECORD_OR_UNION_TYPE_P (instance) || !TYPE_METHODS (instance))
+ if (!RECORD_OR_UNION_TYPE_P (instance))
break;
/* We are interested in concrete template instantiations only: skip
@@ -2442,25 +2432,23 @@ dump_generic_ada_node (pretty_printer *b
static int
print_ada_methods (pretty_printer *buffer, tree node, int spc)
{
- tree t;
- int res;
-
if (!has_nontrivial_methods (node))
return 0;
pp_semicolon (buffer);
- res = 1;
- for (t = TYPE_METHODS (node); t; t = TREE_CHAIN (t))
-{
- if (res)
- {
- pp_newline (buffer);
- pp_newline (buffer);
- }
-
- res = print_ada_declaration (buffer, t, node, spc);
-}
+ int res = 1;
+ for (tree fld = TYPE_FIELDS (node); fld; fld = DECL_CHAIN (fld))
+if (TREE_CODE (TREE_TYPE (fld)) == METHOD_TYPE)
+ {
+ if (res)
+ {
+ pp_newline (buffer);
+ pp_newline (buffer);
+ }
+
+ res = print_ada_declaration (buffer, fld, node, spc);
+ }
return 1;
}
@@ -2961,19 +2949,13 @@ print_ada_declaration (pretty_printer *b
dump_generic_ada_node (buffer, ret_type, type, spc, false, true);
}
- if (is_constructor
- && RECORD_OR_UNION_TYPE_P (type)
- && TYPE_METHODS (type))
- {
- tree tmp;
-
- for (tmp = TYPE_METHODS (type); tmp; tmp = TREE_CHAIN (tmp))
- if (cpp_check (tmp, IS_ABSTRACT))
- {
- is_abstract_class = true;
- break;
- }
- }
+ if (is_constructor && RECORD_OR_UNION_TYPE_P (type))
+ for (tree fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
+ if (cpp_check (fld, IS_ABSTRACT))
+ {
+ is_abstract_class = true;
+ break;
+ }
if (is_abstract || is_abstract_class)
pp_string (buffer, " is abstract");
@@ -3028,35 +3010,33 @@ print_ada_declaration (pretty_printer *b
pp_string (buffer, " is ");
- /* Check whether we have an Ada interface compatible class. */
+ /* Check whether we have an Ada interface compatible class.
+That is only have a vtable non-static data member and no
+non-abstract methods. */
if (cpp_check
- && RECORD_OR_UNION_TYPE_P (TREE_TYPE (t))
- && TYPE_METHODS (TREE_TYPE (t)))
+ && RECORD_OR_UNION_TYPE_P (TREE_TYPE (t)))
{
- int num_fields = 0;
- tree tmp;
+ is_interface = -1;
/* Check that there are no fields other than the virtual table. */
- for (tmp = TYPE_FIELDS (TREE_TYPE (t)); tmp; tmp = TREE_CHAIN (tmp))
+ for (tree fld = TYPE_FIELDS (TREE_TYPE (t));
+ fld; fld = TREE_CHAIN (fld))
{
- if (TREE_CODE (tmp) == TYPE_DECL)
- continue;
- num_fields++;
- }
-
- if (num_fields == 1)
- is_interface
Re: [PATCH] Kill TYPE_METHODS rtl 3/9
This was the most surprising check of TYPE_METHODS. When not optimizing we use
the non-nullness of TYPE_METHODS to figure out if we want to place a non BLKmode
structure into a register. On the grounds that one can't call a member function
with a register-located object.
That seems overly enthusiastic -- if we're not optimizing, who cares?
(When we zap TYHPE_METHODS we currently set it to error_mark_node, if it was
non-null, so that this above check will work).
I'd appreciate comment.
nathan
--
Nathan Sidwell
2017-07-14 Nathan Sidwell
gcc/
* function.c (use_register_for_decl): Always ignore register for
class types when not optimizing.
Index: gcc/function.c
===
--- gcc/function.c (revision 250160)
+++ gcc/function.c (working copy)
@@ -2218,20 +2218,11 @@ use_register_for_decl (const_tree decl)
if (!DECL_REGISTER (decl))
return false;
- switch (TREE_CODE (TREE_TYPE (decl)))
-{
-case RECORD_TYPE:
-case UNION_TYPE:
-case QUAL_UNION_TYPE:
- /* When not optimizing, disregard register keyword for variables with
-types containing methods, otherwise the methods won't be callable
-from the debugger. */
- if (TYPE_METHODS (TYPE_MAIN_VARIANT (TREE_TYPE (decl
- return false;
- break;
-default:
- break;
-}
+ /* When not optimizing, disregard register keyword for types that
+ could have methods, otherwise the methods won't be callable from
+ the debugger. */
+ if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl)))
+return false;
return true;
}
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08
On 07/14/2017 10:34 AM, Segher Boessenkool wrote: > On Thu, Jul 13, 2017 at 10:35:55PM -0600, Jeff Law wrote: There's -fstack-check and -fstack-clash-protection. I think with the direction we're going they are fundamentally incompatible because neither the compiler nor kernel do anything to guarantee enough stack is available after hitting the guard for -fstack-clash-protection. >>> >>> Hrm, I have to think about that. >> Even if the kernel implements the reserved page stuff mentioned earlier >> in the thread, I'm not sure when we'd be able to reliably depend on that >> capability (or even check for it). >> >> ISTM that -fstack-check continues to be Ada centric and we have a new >> option to deal with stack-clash protection and the two simply just >> aren't allowed to be enabled together. > > So this essentially means Ada programs cannot get stack clash protection > at all? And if -fstack-check is really only useful for Ada, we shouldn't > mix up that option with the stack clash thing. It's not a simple yes/no. For Ada the model is that the entire application will be compiled with -fstack-check and that each function that allocates space probes 2-3 pages beyond their immediate need to ensure space to run the signal handler (damn the large register files :-) Those are absolutely critical to keep in mind. In combination they allow prologues to skip the first 2-3 page probes (they were done earlier in the call chain). So from a standpoint of probing each and every allocated page, Ada with -fstack-check will do that if and only if every function is compiled with -fstack-check. But the actual allocation/probing tends to look like this in the prologues allocate all the stack space probe third page probe fourth page probe fifth page etc So there's a race condition between the allocation point and when the probes execute which means an async signal handler could be running on a clashed stack/heap. So an Ada program fully compiled with -fstack-check will mostly be protected from a stack-clash style attack. They could achieve better protection by fixing the -fstack-check=specific prologue sequences so that they allocate and probe a page at a time. That (mostly) closes the issue with signal handlers running on a clashed stack/heap. They could achieve a level of protection in mixed code environments by not skipping page probes. But that would result in significantly worse code given the current implementation of -fstack-check. > > With what is in trunk so far, yeah ;-) In principle the "set up a stack > frame" part of the prologue can be made a separate shrink-wrapping > component and then we'd have to worry. But it would be complicated to > do that and there is no real benefit I see so far, so it won't happen. > Nothing for you to have to worry about in any case :-) That's consistent with my thoughts as well. You could separately wrap the allocation, but it's not likely to be that beneficial and it would seem to add a fair amount of complexity. jeff
Re: [PATCH] Kill TYPE_METHODS ipa 2/9
This patch changes ipa-devirt. It contains a loop checking all the member
functions are 'the same'. However, as the comment says, we've already zapped
TYPE_METHODS, so the loop is never entered. I've added equivalent zapping in
this patch series, as the rationale appears to be reducing memory footprint.
It didn't seem appropriate to merge into the TYPE_FIELDS loop, given it's never
exercised.
I'd appreciate review.
nathan
--
Nathan Sidwell
2017-07-14 Nathan Sidwell
gcc/
* ipa-devirt.c (odr_types_equivalent_p): Delete TYPE_METHODS scan.
Index: gcc/ipa-devirt.c
===
--- gcc/ipa-devirt.c(revision 250160)
+++ gcc/ipa-devirt.c(working copy)
@@ -1602,62 +1602,6 @@ odr_types_equivalent_p (tree t1, tree t2
return false;
}
- if ((TYPE_MAIN_VARIANT (t1) == t1 || TYPE_MAIN_VARIANT (t2) == t2)
- && COMPLETE_TYPE_P (TYPE_MAIN_VARIANT (t1))
- && COMPLETE_TYPE_P (TYPE_MAIN_VARIANT (t2))
- && odr_type_p (TYPE_MAIN_VARIANT (t1))
- && odr_type_p (TYPE_MAIN_VARIANT (t2))
- && (TYPE_METHODS (TYPE_MAIN_VARIANT (t1))
- != TYPE_METHODS (TYPE_MAIN_VARIANT (t2
- {
- /* Currently free_lang_data sets TYPE_METHODS to error_mark_node
- if it is non-NULL so this loop will never realy execute. */
- if (TYPE_METHODS (TYPE_MAIN_VARIANT (t1)) != error_mark_node
- && TYPE_METHODS (TYPE_MAIN_VARIANT (t2)) != error_mark_node)
- for (f1 = TYPE_METHODS (TYPE_MAIN_VARIANT (t1)),
- f2 = TYPE_METHODS (TYPE_MAIN_VARIANT (t2));
- f1 && f2 ; f1 = DECL_CHAIN (f1), f2 = DECL_CHAIN (f2))
- {
- if (DECL_ASSEMBLER_NAME (f1) != DECL_ASSEMBLER_NAME (f2))
- {
- warn_odr (t1, t2, f1, f2, warn, warned,
- G_("a different method of same type "
- "is defined in another "
- "translation unit"));
- return false;
- }
- if (DECL_VIRTUAL_P (f1) != DECL_VIRTUAL_P (f2))
- {
- warn_odr (t1, t2, f1, f2, warn, warned,
- G_("a definition that differs by virtual "
- "keyword in another translation unit"));
- return false;
- }
- if (DECL_VINDEX (f1) != DECL_VINDEX (f2))
- {
- warn_odr (t1, t2, f1, f2, warn, warned,
- G_("virtual table layout differs "
- "in another translation unit"));
- return false;
- }
- if (odr_subtypes_equivalent_p (TREE_TYPE (f1),
-TREE_TYPE (f2), visited,
-loc1, loc2))
- {
- warn_odr (t1, t2, f1, f2, warn, warned,
- G_("method with incompatible type is "
- "defined in another translation unit"));
- return false;
- }
- }
- if ((f1 == NULL) != (f2 == NULL))
- {
- warn_odr (t1, t2, NULL, NULL, warn, warned,
- G_("a type with different number of methods "
-"is defined in another translation unit"));
- return false;
- }
- }
}
break;
}
Re: [PATCH] Kill TYPE_METHODS debug 1/9
This changes dbxout and dwarf2out.
Rather than iterate over the TYPE_METHODS, they now need to deal with member fns
in the regular TYPE_FIELDS iteration.
dbxout was a little weirdly convoluted, apparently presuming that functions with
the same name are all together. That's not true, so other than maybe slight
debug bloat in cases when they happen to be adjacent, it seems more sensible to
handle each member function as a separate item.
The dwarf2out changes are just moving the processing to the TYPE_FIELDS loop,
and thereby deleting some duplicate code.
I'd appreciate a review of this patch.
Oh, the patch series survived a bootstrap on x86_64-linux.
nathan
--
Nathan Sidwell
2017-07-14 Nathan Sidwell
gcc/
* dbxout.c (dbxout_type_fields): Member fns are on TYPE_FIELDS.
(dbxout_type_method_1, dbxout_type_methods): Delete.
(dbxout_type_fn_member): New, constructed from previous.
(dbxout_type): No TYPE_METHODS scan.
* dwarf2out.c (gen_member_die): Member fns are on TYPE_FIELDS.
Index: gcc/dbxout.c
===
--- gcc/dbxout.c(revision 250160)
+++ gcc/dbxout.c(working copy)
@@ -311,8 +311,7 @@ static void dbxout_typedefs (tree);
static void dbxout_type_index (tree);
static void dbxout_args (tree);
static void dbxout_type_fields (tree);
-static void dbxout_type_method_1 (tree);
-static void dbxout_type_methods (tree);
+static void dbxout_type_fn_member (tree);
static void dbxout_range_type (tree, tree, tree);
static void dbxout_type (tree, int);
static bool print_int_cst_bounds_in_octal_p (tree, tree, tree);
@@ -1493,6 +1492,8 @@ dbxout_type_fields (tree type)
|| ! tree_fits_uhwi_p (DECL_SIZE (tem)
continue;
+ else if (TREE_CODE (tem) == FUNCTION_DECL)
+ dbxout_type_fn_member (tem);
else if (TREE_CODE (tem) != CONST_DECL)
{
/* Continue the line if necessary,
@@ -1542,14 +1543,23 @@ dbxout_type_fields (tree type)
}
}
-/* Subroutine of `dbxout_type_methods'. Output debug info about the
- method described DECL. */
+/* Subroutine of `dbxout_type'. Output debug info about the
+ function member DECL. */
static void
-dbxout_type_method_1 (tree decl)
+dbxout_type_fn_member (tree decl)
{
+ if (!use_gnu_debug_info_extensions)
+return;
+
char c1 = 'A', c2;
+ CONTIN;
+ stabstr_I (DECL_NAME (decl));
+ stabstr_S ("::");
+
+ dbxout_type (TREE_TYPE (decl), 0);
+
if (TREE_CODE (TREE_TYPE (decl)) == FUNCTION_TYPE)
c2 = '?';
else /* it's a METHOD_TYPE. */
@@ -1586,72 +1596,7 @@ dbxout_type_method_1 (tree decl)
dbxout_type (DECL_CONTEXT (decl), 0);
stabstr_C (';');
}
-}
-
-/* Subroutine of `dbxout_type'. Output debug info about the methods defined
- in TYPE. */
-
-static void
-dbxout_type_methods (tree type)
-{
- /* C++: put out the method names and their parameter lists */
- tree methods = TYPE_METHODS (type);
- tree fndecl;
- tree last;
-
- if (methods == NULL_TREE)
-return;
-
- if (TREE_CODE (methods) != TREE_VEC)
-fndecl = methods;
- else if (TREE_VEC_ELT (methods, 0) != NULL_TREE)
-fndecl = TREE_VEC_ELT (methods, 0);
- else
-fndecl = TREE_VEC_ELT (methods, 1);
-
- while (fndecl)
-{
- int need_prefix = 1;
-
- /* Group together all the methods for the same operation.
-These differ in the types of the arguments. */
- for (last = NULL_TREE;
- fndecl && (last == NULL_TREE || DECL_NAME (fndecl) == DECL_NAME
(last));
- fndecl = DECL_CHAIN (fndecl))
- /* Output the name of the field (after overloading), as
- well as the name of the field before overloading, along
- with its parameter list */
- {
- /* Skip methods that aren't FUNCTION_DECLs. (In C++, these
-include TEMPLATE_DECLs.) The debugger doesn't know what
-to do with such entities anyhow. */
- if (TREE_CODE (fndecl) != FUNCTION_DECL)
- continue;
-
- CONTIN;
-
- last = fndecl;
-
- /* Also ignore abstract methods; those are only interesting to
-the DWARF backends. */
- if (DECL_IGNORED_P (fndecl) || DECL_ABSTRACT_P (fndecl))
- continue;
-
- /* Redundantly output the plain name, since that's what gdb
-expects. */
- if (need_prefix)
- {
- stabstr_I (DECL_NAME (fndecl));
- stabstr_S ("::");
- need_prefix = 0;
- }
-
- dbxout_type (TREE_TYPE (fndecl), 0);
- dbxout_type_method_1 (fndecl);
- }
- if (!need_prefix)
- stabstr_C (';');
-}
+ stabstr_C (';');
}
/* Emit a "range" type specification, which has the form:
@@ -2211,10 +2156,6 @@ dbxout_type (tree type, int full)
/* Write out the field declarations. */
dbxout_type_fields (type);
- if (use_gnu_debug_info
[PATCH] Kill TYPE_METHODS 0/9
This is a series of patches that remove the TYPE_METHODS field used in records & unions. Currently TYPE_METHODS hods a the member functions (be they static or non-static), and TYPE_FIELDS holds everything else (be they FIELD_DECLS or whatever). This distinction is unnecessary, and the patches move everything to TYPE_FIELDS. (I do not mess with name lookup, which is handled differently). I do not repurpose TYPE_METHODS, that's later. While the changes are pretty mechanical, some are rather too large outside of the C++ FE to comfortably apply the obvious rule. 1 method-debug.diff - dbxout & dwarf2out. Review please. 2 method-ipa.diff - lto-devirt. Review please. 3 method-rtl.diff - most odd occurrence. Comment please. 4 method-ada.diff - ada-spec generation. Obvious. 5 method-cp.diff - C++ FE changes, self reviewed 6 method-libcc1.diff - libcp1plugin. Obvious. 7 method-misc.diff - random tree.c. Obvious. 8 method-objc.diff - objc. Obvious 9 method-ectomy.diff - delete the macro. Obvious nathan -- Nathan Sidwell
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08
On Thu, Jul 13, 2017 at 10:35:55PM -0600, Jeff Law wrote: > >> There's -fstack-check and -fstack-clash-protection. I think with the > >> direction we're going they are fundamentally incompatible because > >> neither the compiler nor kernel do anything to guarantee enough stack is > >> available after hitting the guard for -fstack-clash-protection. > > > > Hrm, I have to think about that. > Even if the kernel implements the reserved page stuff mentioned earlier > in the thread, I'm not sure when we'd be able to reliably depend on that > capability (or even check for it). > > ISTM that -fstack-check continues to be Ada centric and we have a new > option to deal with stack-clash protection and the two simply just > aren't allowed to be enabled together. So this essentially means Ada programs cannot get stack clash protection at all? And if -fstack-check is really only useful for Ada, we shouldn't mix up that option with the stack clash thing. > >> And there's shrink wrapping. This was originally raised by the aarch64 > >> guys and deserves some thought. I'm particularly worried about aarch64 > >> if it was to shrink-wrap some particular register saves that we wanted > >> to use as an implicit probe. > > > > For normal shrink-wrapping, the volatile reg stores will always happen > > in the same prologue that sets up the stack frame as well, so there > > won't be any problem (except the usual stack ties you might need in the > > prologue, etc.) (*) For separate shrink-wrapping, yeah you'll need any > > bb that could potentially store to the stack require the component for > > the register you use as implicit probe. This is pretty nasty. > I'm going to try and look at this tomorrow to get a feel for the aarch64 > implementation separate shrink wrapping. > > For ppc I'm not too worried -- the implicit backchain probes are so > effective at eliminating explicit probes that I didn't bother writing > any code to track the register saves as implicit probes. I'm pretty > sure it just works on ppc with separate shrink wrapping. With what is in trunk so far, yeah ;-) In principle the "set up a stack frame" part of the prologue can be made a separate shrink-wrapping component and then we'd have to worry. But it would be complicated to do that and there is no real benefit I see so far, so it won't happen. Nothing for you to have to worry about in any case :-) Segher
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 02/08
On 07/13/2017 04:54 PM, Jeff Law wrote: > On 07/12/2017 07:44 PM, Segher Boessenkool wrote: >>> /* The default is not to move the stack pointer. */ >>> +/* The default is not to move the stack pointer, unless we are using >>> + stack clash prevention stack checking. */ >>> #ifndef STACK_CHECK_MOVING_SP >>> -#define STACK_CHECK_MOVING_SP 0 >>> +#define STACK_CHECK_MOVING_SP\ >>> + (flag_stack_check == STACK_CLASH_BUILTIN_STACK_CHECK) >>> #endif >> >> Missing space before that backslash. > Similarly. > >> >> The documentation for STACK_CHECK_CONFIG_SP needs updating (its default >> is no longer zero, for one). > Yea. Missed that. I actually need to go back and look at this again. > I'm not entirely sure it's necessary -- it may be a relic from when I > thought more -fstack-check infrastructure was going to be reusable. Just a follow-up to myself. As I suspected, we can just drop the STACK_CHECK_MOVING_SP changes. Between changes in overall direction and moving stack-clash protection out of -fstack-check= STACK_CHECK_MOVING_SP changes simply aren't needed anymore. jeff
Re: [PING] C++ Re: [PATCH] C/C++: fix quoting of "aka" typedef information (PR 62170)
On 06/21/2017 01:59 AM, Volker Reichelt wrote:
On 20 Jun, Jason Merrill wrote:
On Tue, Jun 20, 2017 at 3:06 PM, David Malcolm wrote:
It's not clear to me what the issue alluded to with negative
obstack_blank is, but I chose to follow the above docs and use
obstack_blank_fast; am testing an updated patch in which the above line
now looks like:
obstack_blank_fast (ob, -(type_start + type_len));
Is the patch OK with that change? (assuming bootstrap®rtesting
pass), or should I re-post?
OK with that change.
On a related matter, this patch conflicts with Volker's patch here:
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01576.html
in which he removes the trailing "{enum}" info (and hence all of our
changes to the testsuite conflict between the two patches...)
Do you have any thoughts on that other patch? [Ccing Volker]
That patch makes sense to me; I prefer "enum E" to "E {enum}".
Jason
Is 'makes sense' equivalent to 'OK for trunk' here? If so, should my
patch go in before David's or should we do it the other way round?
I missed this and have been pinging your patch on your behalf
(below). In the interest on making progress on this, IMO trivial,
change I recommend taking Jason's comment as approval.
https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00472.html
Martin
[PING] [PATCH] v3: C/C++: fix quoting of "aka" typedef information (PR 62170)
Jason, is this updated version of the patch OK? https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01580.html Thanks Dave On Wed, 2017-06-21 at 10:32 -0400, David Malcolm wrote: > On Tue, 2017-06-20 at 15:11 -0400, Jason Merrill wrote: > > On Tue, Jun 20, 2017 at 3:06 PM, David Malcolm > > > > wrote: > > > It's not clear to me what the issue alluded to with negative > > > obstack_blank is, but I chose to follow the above docs and use > > > obstack_blank_fast; am testing an updated patch in which the > > > above > > > line > > > now looks like: > > > > > > obstack_blank_fast (ob, -(type_start + type_len)); > > > > > > Is the patch OK with that change? (assuming bootstrap®rtesting > > > pass), or should I re-post? > > > > OK with that change. > > It turns out that the resizing calculation in the above line is > wrong, but I managed not to expose my mistake in the light testing > I did before posting the above. Oops. > > Thankfully the issue showed up in g++.dg/other/error23.C when I > ran the full test suite. > > Sorry about that. > > I've updated the shrinking calculation to look like this: > > int delta = type_start + type_len - obstack_object_size (ob); > gcc_assert (delta <= 0); > obstack_blank_fast (ob, delta); > > and with that, it successfully bootstrapped & regression-tested on > x86_64-pc-linux-gnu. > > OK for trunk? > > Full patch follows, for reference: > > gcc/c/ChangeLog: > PR c++/62170 > * c-objc-common.c (c_tree_printer): Convert penultimate param > from > bool to bool *. Within '%T' handling, if showing an "aka", use > "quoted" param to add appropriate quoting. > > gcc/cp/ChangeLog: > PR c++/62170 > * error.c (type_to_string): Add leading comment. Add params > "postprocessed", "quote", and "show_color", using them to fix > quoting of the "aka" for types involving typedefs. > (arg_to_string): Update for new params to type_to_string. > (cxx_format_postprocessor::handle): Likewise. > (cp_printer): Convert penultimate param from bool to bool *. > Update call to type_to_string and calls to > defer_phase_2_of_type_diff. > > gcc/fortran/ChangeLog: > PR c++/62170 > * error.c (gfc_notify_std): Convert "quoted" param from bool to > bool *. > > gcc/ChangeLog: > PR c++/62170 > * pretty-print.c (pp_format): Move quoting implementation to > pp_begin_quote and pp_end_quote. Update pp_format_decoder call > to pass address of "quote" local. > (pp_begin_quote): New function. > (pp_end_quote): New function. > * pretty-print.h (printer_fn): Convert penultimate param from > bool > to bool *. > (pp_begin_quote): New decl. > (pp_end_quote): New decl. > * tree-diagnostic.c (default_tree_printer): Convert penultimate > param from bool to bool *. > * tree-diagnostic.h (default_tree_printer): Likewise. > > gcc/testsuite/ChangeLog: > PR c++/62170 > * g++.dg/cpp1z/direct-enum-init1.C: Update expected error > messages > to reflect fixes to quoting. > * g++.dg/diagnostic/aka1.C: Likewise. > * g++.dg/diagnostic/aka2.C: New test case. > * g++.dg/parse/error55.C: Update expected error messages to > reflect fixes to quoting. > * g++.dg/warn/pr12242.C: Likewise. > * g++.old-deja/g++.mike/enum1.C: Likewise. > * gcc.dg/diag-aka-1.c: Likewise. > * gcc.dg/diag-aka-2.c: New test case. > * gcc.dg/pr13804-1.c: Update expected error messages to reflect > fixes to quoting. > * gcc.dg/pr56980.c: Likewise. > * gcc.dg/pr65050.c: Likewise. > * gcc.dg/redecl-14.c: Likewise. > * gcc.dg/utf16-4.c Likewise. > * gcc.target/i386/sse-vect-types.c (__m128d): Likewise. > * obj-c++.dg/invalid-type-1.mm: Likewise. > --- > gcc/c/c-objc-common.c | 12 +- > gcc/cp/error.c | 94 -- > gcc/fortran/error.c| 2 +- > gcc/pretty-print.c | 37 +++- > gcc/pretty-print.h | 5 +- > gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C | 234 --- > -- > gcc/testsuite/g++.dg/diagnostic/aka1.C | 2 +- > gcc/testsuite/g++.dg/diagnostic/aka2.C | 32 > gcc/testsuite/g++.dg/parse/error55.C | 2 +- > gcc/testsuite/g++.dg/warn/pr12242.C| 16 +- > gcc/testsuite/g++.old-deja/g++.mike/enum1.C| 2 +- > gcc/testsuite/gcc.dg/diag-aka-1.c | 4 +- > gcc/testsuite/gcc.dg/diag-aka-2.c | 12 ++ > gcc/testsuite/gcc.dg/pr13804-1.c | 4 +- > gcc/testsuite/gcc.dg/pr56980.c | 12 +- > gcc/testsuite/gcc.dg/pr65050.c | 8 +- > gcc/testsuite/gcc.dg/redecl-14.c | 2 +- > gcc/testsuite/gcc.dg/utf16-4.c | 2 +- > gcc/testsuite
[Patch ARM] Document the +crypto extension on CPUs.
Hi,
We don't document the list of CPU names which can take a +crypto extension
in the ARM port. This patch fixes that oversight.
OK?
Thanks,
James
---
2017-14-07 James Greenhalgh
* doc/invoke.texi (arm/-mcpu): Document +crypto.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 28070a6..56667fc 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -15636,6 +15636,14 @@ on @samp{cortex-r5} and @samp{cortex-m7}.
Disables the SIMD (but not floating-point) instructions on
@samp{generic-armv7-a}, @samp{cortex-a5}, @samp{cortex-a7}
and @samp{cortex-a9}.
+
+@item +crypto
+Enables the cryptographic instructions on @samp{cortex-a32},
+@samp{cortex-a35}, @samp{cortex-a53}, @samp{cortex-a55}, @samp{cortex-a57},
+@samp{cortex-a72}, @samp{cortex-a73}, @samp{cortex-a75}, @samp{exynos-m1},
+@samp{xgene1}, @samp{cortex-a57.cortex-a53}, @samp{cortex-a72.cortex-a53},
+@samp{cortex-a73.cortex-a35}, @samp{cortex-a73.cortex-a53} and
+@samp{cortex-a75.cortex-a55}.
@end table
Additionally the @samp{generic-armv7-a} pseudo target defaults to
Re: [Patch ARM] Document the +crypto extension on CPUs.
On 14/07/17 16:44, James Greenhalgh wrote: Hi, We don't document the list of CPU names which can take a +crypto extension in the ARM port. This patch fixes that oversight. OK? Ok. Thanks, Kyrill Thanks, James --- 2017-14-07 James Greenhalgh * doc/invoke.texi (arm/-mcpu): Document +crypto.
Re: C PATCH to display types when printing a conversion warning (PR c/81233)
On 07/14/2017 07:47 AM, Marek Polacek wrote: On Fri, Jul 14, 2017 at 02:52:36PM +0200, Marek Polacek wrote: On Thu, Jul 13, 2017 at 11:42:15AM -0600, Martin Sebor wrote: On 07/13/2017 08:18 AM, Marek Polacek wrote: This patch improves diagnostic in the C FE by printing the types when reporting a problem with a conversion. E.g., instead of warning: assignment from incompatible pointer type you'll now get warning: assignment to 'int *' from incompatible pointer type 'char *' or instead of warning: initialization makes integer from pointer without a cast this warning: initialization of 'int *' from 'int' makes pointer from integer without a cast I've been wanting this for a long time and here it is. Two snags: I had to make pedwarn_init to take '...' for which I had to introduce emit_diagnostic_valist; you can't pass varargs from one vararg function to another vararg function (and a macro with __VA_ARGS__ didn't work here). Also, PEDWARN_FOR_ASSIGNMENT didn't work with the addition of printing TYPE and RHSTYPE so I just decided to unroll the macro instead of making it even more ugly. This patch is long but it's mainly because of the testsuite fallout. If you have better ideas about the wording, let me know. It looks pretty good as is. My only wording suggestion is to consider simply mentioning conversion in the text of the warnings: warning: conversion to T* from an incompatible type U* I'm not sure that being explicit about the context where the bad conversion takes place (initialization vs assignment vs returning a value) is terribly helpful. That would not only simplify the code and make all the messages consistent, but it would also make it possible to get rid of the note when passing arguments. Yeah, I agree, actually. We print the expressions in question (although, we could probably do even better), and I don't see why it would be necessary to mention whether it's an initialization or an assignment. I think I'll just drop this patch and do something along the lines you suggest. David, do you agree with this? (Joseph's on PTO but I'd of course like to hear his opinion, too.) I think I changed my mind. Because clang says e.g. warning: returning 'unsigned int *' from a function with result type 'int *' converts between pointers to integer types with different sign so in the end it might be best to just go with my current patch; it should only improve things anyway. And then add the fix-it hint. Yes, Clang does do that. G++, OTOH, prints an error with the same text in all these cases. It's not tremendously important which of the two forms is used. What I do think would be nice is if the text of the same diagnostics, whether warnings or errors, could be more consistent between the front ends. A good way to do that in general, if all of the checking code cannot be shared, is to provide a shared diagnose_this_or_that() function for each diagnostic. The function would be parameterized on the kind of diagnostic (i.e., error or warning), but would hardcode the shared text. Each FE would call it with an argument telling it whether to issue it as an error or warning. Martin One more thing: for int *q = p; int i = q; we should be able to provide a fix-it hint, something like 'did you mean to dereference q?'. With the current PEDWARN_FOR_ASSIGNMENT macro it would be awkward to implement that. There are still more warnings to improve but I think better to do this incrementally rather than a single humongous patch. That makes sense. I was going to mention that it would be nice to also improve: warning: comparison of distinct pointer types lacks a cast If you take the conversion suggestion I think this warning would need to be phrased in terms of "conversion between T* and U*" rather than "conversion from T* to U*". (A similar change could be made to the error message printed when incompatible pointers are subtracted from one another.) Sure. Marek Marek
Re: [PATCH, GCC/ARM] Rewire -mfpu=fp-armv8 as VFPv5 + D32 + DP
Hi Richard, I've committed the requested change as a separate patch to make it easier to backport to earlier GCC versions. Definition of __ARM_FEATURE_NUMERIC_MAXMIN checks for TARGET_ARM_ARCH >= 8 and TARGET_NEON being true in addition to TARGET_VFP5. However, instructions covered by this macro are part of FPv5 which is available in ARMv7E-M architecture. This patch fixes the macro to only check for TARGET_VFP5. ChangeLog entry is as follows: *** gcc/ChangeLog *** * config/arm/arm-c.c (arm_cpu_builtins): Define __ARM_FEATURE_NUMERIC_MAXMIN solely based on TARGET_VFP5. Built and confirmed that the macro is now defined when building with -march=armv7e-m+fpv5 -mfloat-abi=hard. Best regards, Thomas On 14/07/17 15:43, Richard Earnshaw (lists) wrote: On 14/07/17 09:20, Thomas Preudhomme wrote: Hi, fp-armv8 is currently defined as a double precision FPv5 with 32 D registers *and* a special FP_ARMv8 bit. However FP for ARMv8 should only bring 32 D registers on top of FPv5-D16 so this FP_ARMv8 bit is spurious. As a consequence, many instruction patterns which are guarded by TARGET_FPU_ARMV8 are unavailable to FPv5-D16 and FPv5-SP-D16. This patch gets rid of TARGET_FPU_ARMV8 and rewire all uses to expressions based on TARGET_VFP5, TARGET_VFPD32 and TARGET_VFP_DOUBLE. It also redefine ISA_FP_ARMv8 to include the D32 capability to distinguish it from FPv5-D16. At last, it sets the +fp.sp for ARMv8-R to enable FPv5-SP-D16 (ie FP for ARMv8 with single precision only and 16 D registers). ChangeLog entry is as follows: 2017-07-07 Thomas Preud'homme * config/arm/arm-isa.h (isa_bit_FP_ARMv8): Delete enumerator. (ISA_FP_ARMv8): Define as ISA_FPv5 and ISA_FP_D32. * config/arm/arm-cpus.in (armv8-r): Define fp.sp as enabling FPv5. (fp-armv8): Define it as FP_ARMv8 only. config/arm/arm.h (TARGET_FPU_ARMV8): Delete. (TARGET_VFP_FP16INST): Define using TARGET_VFP5 rather than TARGET_FPU_ARMV8. config/arm/arm.c (arm_rtx_costs_internal): Replace checks against TARGET_FPU_ARMV8 by checks against TARGET_VFP5. * config/arm/arm-builtins.c (arm_builtin_vectorized_function): Define first ARM_CHECK_BUILTIN_MODE definition using TARGET_VFP5 rather than TARGET_FPU_ARMV8. * config/arm/arm-c.c (arm_cpu_builtins): Likewise for __ARM_FEATURE_NUMERIC_MAXMIN macro definition. * config/arm/arm.md (cmov): Condition on TARGET_VFP5 rather than TARGET_FPU_ARMV8. * config/arm/neon.md (neon_vrint): Likewise. (neon_vcvt): Likewise. (neon_): Likewise. (3): Likewise. * config/arm/vfp.md (lsi2): Likewise. * config/arm/predicates.md (arm_cond_move_operator): Check against TARGET_VFP5 rather than TARGET_FPU_ARMV8 and fix spacing. Testing: * Bootstrapped under ARMv8-A Thumb state and ran testsuite -> no regression * built Spec2000 and Spec2006 with -march=armv8-a+fp16 and compared objdump -> no code generation difference Is this ok for trunk? OK with changes mentioned below. R. Best regards, Thomas rewire_mfpu_fparmv8.patch diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c index 63ee880822c17eda55dd58438d61cbbba333b2c6..7504ed581c63a657a0dff48442633704bd252b2e 100644 --- a/gcc/config/arm/arm-builtins.c +++ b/gcc/config/arm/arm-builtins.c @@ -3098,7 +3098,7 @@ arm_builtin_vectorized_function (unsigned int fn, tree type_out, tree type_in) NULL_TREE is returned if no such builtin is available. */ #undef ARM_CHECK_BUILTIN_MODE #define ARM_CHECK_BUILTIN_MODE(C)\ - (TARGET_FPU_ARMV8 \ + (TARGET_VFP5 \ && flag_unsafe_math_optimizations \ && ARM_CHECK_BUILTIN_MODE_1 (C)) diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c index a3daa3220a2bc4220dffdb7ca08ca9419bdac425..9178937b6d9e0fe5d0948701390c4cf01f4f8c7d 100644 --- a/gcc/config/arm/arm-c.c +++ b/gcc/config/arm/arm-c.c @@ -96,7 +96,7 @@ arm_cpu_builtins (struct cpp_reader* pfile) || TARGET_ARM_ARCH_ISA_THUMB >=2)); def_or_undef_macro (pfile, "__ARM_FEATURE_NUMERIC_MAXMIN", - TARGET_ARM_ARCH >= 8 && TARGET_NEON && TARGET_FPU_ARMV8); + TARGET_ARM_ARCH >= 8 && TARGET_NEON && TARGET_VFP5); This looks wrong (though ACLE is misleading). The MAXMIN property is solely defined by having an FPv5 capable FPU. def_or_undef_macro (pfile, "__ARM_FEATURE_SIMD32", TARGET_INT_SIMD); diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in index f35128acb7d68c6a0592355b9d3d56ee8f826aca..e2ff297aed7514073dbb3bf5ee86964f202e5a14 100644 --- a/gcc/config/arm/arm-cpus.in +++ b/gcc/config/arm/arm-cpus.in @@ -389,7 +389,7 @@ begin arch armv8-r option crc add bit_crc32 # fp.sp => fp-armv8 (d16); simd => simd + fp-armv8 + d32 + double precision Please update comment # note: no fp option for fp-armv8 (d16) + double precision at the moment - option fp.sp add FP_ARMv8 + option fp.sp add FPv5 op
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 08/08
On 07/14/2017 08:29 AM, Andreas Krebbel wrote: > On 07/11/2017 11:21 PM, Jeff Law wrote: >> This patch adds s390 support for stack-clash mitigation. >> >> s390's most interesting property is that the caller allocates space for >> the callee to save registers into. >> >> So much like aarch64, we start with a very conservative assumption about >> the offset between SP and the most recent stack probe. As we encounter >> those register saves we may be able to decrease that offset. And like >> aarch64 as we allocate space, the offset increases. If the offset >> crosses PROBE_INTERVAL, we must emit probes. >> >> Because the register saves hit the caller's frame s390 in some ways >> generates code more like x86/ppc. Though if there aren't any register >> saves, then the resulting code looks more like aarch64. >> >> For large frames, I did not implement an allocate/probe in a loop. >> Someone with a better understanding of the architecture is better suited >> for that work. I'll note that you're going to need another scratch >> register :-) This is the cause of the xfail of one test which expects >> to see a prologue allocate/probe loop. >> >> s390 has a -mbackchain option. I'm not sure where it's used, but we do >> try to handle it in the initial offset computation. However, we don't >> handle it in the actual allocations that occur when -fstack-check=clash >> is enabled. >> >> s390 does not have a -fstack-check=specific implementation. I have not >> tried to add one. But I have defined STACK_CHECK_STATIC_BUILTIN. I >> haven't investigated what side effects that might have. >> >> Other than the xfail noted above, the s390 uses the same tests as the >> x86, ppc and aarch64 ports. >> >> I suspect we're going to need further iteration here. >> >> Thoughts/Comments? > > I'll have a look and run some tests to come back with an answer next week. Thanks. That'd be greatly appreciated. Jeff
Re: [PATCH, GCC/ARM] Rewire -mfpu=fp-armv8 as VFPv5 + D32 + DP
On 14/07/17 09:20, Thomas Preudhomme wrote: > Hi, > > fp-armv8 is currently defined as a double precision FPv5 with 32 D > registers *and* a special FP_ARMv8 bit. However FP for ARMv8 should only > bring 32 D registers on top of FPv5-D16 so this FP_ARMv8 bit is > spurious. As a consequence, many instruction patterns which are guarded > by TARGET_FPU_ARMV8 are unavailable to FPv5-D16 and FPv5-SP-D16. > > This patch gets rid of TARGET_FPU_ARMV8 and rewire all uses to > expressions based on TARGET_VFP5, TARGET_VFPD32 and TARGET_VFP_DOUBLE. > It also redefine ISA_FP_ARMv8 to include the D32 capability to > distinguish it from FPv5-D16. At last, it sets the +fp.sp for ARMv8-R to > enable FPv5-SP-D16 (ie FP for ARMv8 with single precision only and 16 D > registers). > > ChangeLog entry is as follows: > > 2017-07-07 Thomas Preud'homme > > * config/arm/arm-isa.h (isa_bit_FP_ARMv8): Delete enumerator. > (ISA_FP_ARMv8): Define as ISA_FPv5 and ISA_FP_D32. > * config/arm/arm-cpus.in (armv8-r): Define fp.sp as enabling FPv5. > (fp-armv8): Define it as FP_ARMv8 only. > config/arm/arm.h (TARGET_FPU_ARMV8): Delete. > (TARGET_VFP_FP16INST): Define using TARGET_VFP5 rather than > TARGET_FPU_ARMV8. > config/arm/arm.c (arm_rtx_costs_internal): Replace checks against > TARGET_FPU_ARMV8 by checks against TARGET_VFP5. > * config/arm/arm-builtins.c (arm_builtin_vectorized_function): Define > first ARM_CHECK_BUILTIN_MODE definition using TARGET_VFP5 rather > than TARGET_FPU_ARMV8. > * config/arm/arm-c.c (arm_cpu_builtins): Likewise for > __ARM_FEATURE_NUMERIC_MAXMIN macro definition. > * config/arm/arm.md (cmov): Condition on TARGET_VFP5 rather than > TARGET_FPU_ARMV8. > * config/arm/neon.md (neon_vrint): Likewise. > (neon_vcvt): Likewise. > (neon_): Likewise. > (3): Likewise. > * config/arm/vfp.md (lsi2): Likewise. > * config/arm/predicates.md (arm_cond_move_operator): Check against > TARGET_VFP5 rather than TARGET_FPU_ARMV8 and fix spacing. > > Testing: > * Bootstrapped under ARMv8-A Thumb state and ran testsuite -> no > regression > * built Spec2000 and Spec2006 with -march=armv8-a+fp16 and compared > objdump -> no code generation difference > > Is this ok for trunk? OK with changes mentioned below. R. > > Best regards, > > Thomas > > rewire_mfpu_fparmv8.patch > > > diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c > index > 63ee880822c17eda55dd58438d61cbbba333b2c6..7504ed581c63a657a0dff48442633704bd252b2e > 100644 > --- a/gcc/config/arm/arm-builtins.c > +++ b/gcc/config/arm/arm-builtins.c > @@ -3098,7 +3098,7 @@ arm_builtin_vectorized_function (unsigned int fn, tree > type_out, tree type_in) > NULL_TREE is returned if no such builtin is available. */ > #undef ARM_CHECK_BUILTIN_MODE > #define ARM_CHECK_BUILTIN_MODE(C)\ > - (TARGET_FPU_ARMV8 \ > + (TARGET_VFP5 \ > && flag_unsafe_math_optimizations \ > && ARM_CHECK_BUILTIN_MODE_1 (C)) > > diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c > index > a3daa3220a2bc4220dffdb7ca08ca9419bdac425..9178937b6d9e0fe5d0948701390c4cf01f4f8c7d > 100644 > --- a/gcc/config/arm/arm-c.c > +++ b/gcc/config/arm/arm-c.c > @@ -96,7 +96,7 @@ arm_cpu_builtins (struct cpp_reader* pfile) > || TARGET_ARM_ARCH_ISA_THUMB >=2)); > >def_or_undef_macro (pfile, "__ARM_FEATURE_NUMERIC_MAXMIN", > - TARGET_ARM_ARCH >= 8 && TARGET_NEON && TARGET_FPU_ARMV8); > + TARGET_ARM_ARCH >= 8 && TARGET_NEON && TARGET_VFP5); This looks wrong (though ACLE is misleading). The MAXMIN property is solely defined by having an FPv5 capable FPU. > >def_or_undef_macro (pfile, "__ARM_FEATURE_SIMD32", TARGET_INT_SIMD); > > diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in > index > f35128acb7d68c6a0592355b9d3d56ee8f826aca..e2ff297aed7514073dbb3bf5ee86964f202e5a14 > 100644 > --- a/gcc/config/arm/arm-cpus.in > +++ b/gcc/config/arm/arm-cpus.in > @@ -389,7 +389,7 @@ begin arch armv8-r > option crc add bit_crc32 > # fp.sp => fp-armv8 (d16); simd => simd + fp-armv8 + d32 + double precision Please update comment > # note: no fp option for fp-armv8 (d16) + double precision at the moment > - option fp.sp add FP_ARMv8 > + option fp.sp add FPv5 > option simd add FP_ARMv8 NEON > option crypto add FP_ARMv8 CRYPTO > option nocrypto remove ALL_CRYPTO > @@ -1390,7 +1390,7 @@ begin fpu fpv5-d16 > end fpu fpv5-d16 > > begin fpu fp-armv8 > - isa FP_ARMv8 FP_D32 > + isa FP_ARMv8 > end fpu fp-armv8 > > begin fpu neon-fp-armv8 > diff --git a/gcc/config/arm/arm-isa.h b/gcc/config/arm/arm-isa.h > index > 0d66a0400c517668db023fc66ff43e26d43add51..dbd29eaa52f2007498c2aff6263b8b6c3a70e2c2 > 100644 > --- a/gcc/config/arm/arm-isa.h > +++ b/gcc/config/arm/arm-isa.h > @@ -60,7 +60,6 @@ enum isa_feature > isa_bit_VFPv4, /* Vector floating point v4. */ > isa_bit_FPv5,
[PATCH PR81369/02]Conservatively not distribute loop with unknown niters
Hi,
This is a followup patch for previous fix to PR81369. In that test case, GCC
tries to distribute infinite loop, which doesn't make much sense. This patch
works conservatively by skipping loops with unknown niters. It also simplifies
code a bit.
Bootstrap and test on x86_64 and AArch64, is it OK?
Thanks,
bin
2017-07-12 Bin Cheng
PR target/81369
* tree-loop-distribution.c (classify_partition): Only assert on
numer of iterations.
(merge_dep_scc_partitions): Delete prameter. Update function call.
(distribute_loop): Remove code handling loop with unknown niters.
(pass_loop_distribution::execute): Skip loop with unknown niters.From b96c0053b79fd457df1fdb91c4401a1a7ccace7d Mon Sep 17 00:00:00 2001
From: Bin Cheng
Date: Wed, 12 Jul 2017 12:30:12 +0100
Subject: [PATCH 2/2] skip-loop-with-unknown-niters.txt
---
gcc/tree-loop-distribution.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/gcc/tree-loop-distribution.c b/gcc/tree-loop-distribution.c
index fe678a5..497e6a9 100644
--- a/gcc/tree-loop-distribution.c
+++ b/gcc/tree-loop-distribution.c
@@ -1412,8 +1412,7 @@ classify_partition (loop_p loop, struct graph *rdg, partition *partition,
return;
nb_iter = number_of_latch_executions (loop);
- if (!nb_iter || nb_iter == chrec_dont_know)
-return;
+ gcc_assert (nb_iter && nb_iter != chrec_dont_know);
if (dominated_by_p (CDI_DOMINATORS, single_exit (loop)->src,
gimple_bb (DR_STMT (single_store
plus_one = true;
@@ -1962,18 +1961,16 @@ sort_partitions_by_post_order (struct graph *pg,
}
/* Given reduced dependence graph RDG merge strong connected components
- of PARTITIONS. If IGNORE_ALIAS_P is true, data dependence caused by
- possible alias between references is ignored, as if it doesn't exist
- at all; otherwise all depdendences are considered. */
+ of PARTITIONS. In this function, data dependence caused by possible
+ alias between references is ignored, as if it doesn't exist at all. */
static void
merge_dep_scc_partitions (struct graph *rdg,
- vec *partitions,
- bool ignore_alias_p)
+ vec *partitions)
{
struct partition *partition1, *partition2;
struct pg_vdata *data;
- graph *pg = build_partition_graph (rdg, partitions, ignore_alias_p);
+ graph *pg = build_partition_graph (rdg, partitions, true);
int i, j, num_sccs = graphds_scc (pg, NULL);
/* Strong connected compoenent means dependence cycle, we cannot distribute
@@ -2420,9 +2417,6 @@ distribute_loop (struct loop *loop, vec stmts,
auto_vec partitions;
rdg_build_partitions (rdg, stmts, &partitions);
- /* Can't do runtime alias check if loop niter is unknown. */
- tree niters = number_of_latch_executions (loop);
- bool rt_alias_check_p = (niters != NULL_TREE && niters != chrec_dont_know);
auto_vec alias_ddrs;
auto_bitmap stmt_in_all_partitions;
@@ -2511,9 +2505,9 @@ distribute_loop (struct loop *loop, vec stmts,
/* Build the partition dependency graph. */
if (partitions.length () > 1)
{
- merge_dep_scc_partitions (rdg, &partitions, rt_alias_check_p);
+ merge_dep_scc_partitions (rdg, &partitions);
alias_ddrs.truncate (0);
- if (rt_alias_check_p && partitions.length () > 1)
+ if (partitions.length () > 1)
break_alias_scc_partitions (rdg, &partitions, &alias_ddrs);
}
@@ -2653,6 +2647,11 @@ pass_loop_distribution::execute (function *fun)
if (!optimize_loop_for_speed_p (loop))
continue;
+ /* Don't distribute loop if niters is unknown. */
+ tree niters = number_of_latch_executions (loop);
+ if (niters == NULL_TREE || niters == chrec_dont_know)
+ continue;
+
/* Initialize the worklist with stmts we seed the partitions with. */
bbs = get_loop_body_in_dom_order (loop);
for (i = 0; i < loop->num_nodes; ++i)
--
1.9.1
[PATCH PR81369/01]Sort partitions by post order for all cases
Hi,
This patch fixes ICE reported by PR81369. It simply sinks call to
sort_partitions_by_post_order so that it's executed for all cases.
This is necessary to schedule reduction partition as the last one.
Bootstrap and test on x86_64 and AArch64. Is it OK?
Thanks,
bin
2017-07-12 Bin Cheng
PR target/81369
* tree-loop-distribution.c (merge_dep_scc_partitions): Sink call to
function sort_partitions_by_post_order.
gcc/testsuite/ChangeLog
2017-07-12 Bin Cheng
PR target/81369
* gcc.dg/tree-ssa/pr81369.c: New.From 685bab237e38544375dcc9a950ae2816b8e4385b Mon Sep 17 00:00:00 2001
From: Bin Cheng
Date: Wed, 12 Jul 2017 12:04:49 +0100
Subject: [PATCH 1/2] pr81369-1.txt
---
gcc/testsuite/gcc.dg/tree-ssa/pr81369.c | 23 +++
gcc/tree-loop-distribution.c| 3 ++-
2 files changed, 25 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr81369.c
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr81369.c b/gcc/testsuite/gcc.dg/tree-ssa/pr81369.c
new file mode 100644
index 000..b40477b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr81369.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-loop-distribution" } */
+
+typedef __PTRDIFF_TYPE__ intptr_t;
+int wo;
+
+void
+sy (long int *as)
+{
+ for (;;)
+{
+ *as = wo;
+ while (as < (long int *) (void *) 2)
+{
+ int *y9;
+
+ if (wo != 0)
+*y9 = (int) (intptr_t) &wo;
+ wo /= (wo != 0 && *y9 != 0);
+ ++as;
+}
+}
+}
diff --git a/gcc/tree-loop-distribution.c b/gcc/tree-loop-distribution.c
index be0a660..fe678a5 100644
--- a/gcc/tree-loop-distribution.c
+++ b/gcc/tree-loop-distribution.c
@@ -1997,8 +1997,9 @@ merge_dep_scc_partitions (struct graph *rdg,
data->partition = NULL;
}
}
- sort_partitions_by_post_order (pg, partitions);
}
+
+ sort_partitions_by_post_order (pg, partitions);
gcc_assert (partitions->length () == (unsigned)num_sccs);
free_partition_graph_vdata (pg);
free_graph (pg);
--
1.9.1
Re: [RFC][PATCH][AArch64] Cleanup frame pointer usage
ping
Wilco Dijkstra wrote:
> James Greenhalgh wrote:
>
> > I note this is still marked as an RFC, are you now proposing it as a
> > patch to be merged to trunk?
>
> Absolutely. It was marked as an RFC to get some comments - I thought it
> may be controversial to separate the frame pointer and frame chain concept.
> And this fixes the long standing bugs caused by changing the global frame
> pointer option to an incorrect value for the leaf function optimization.
Here is a rebased version that should patch without merge issues:
Cleanup frame pointer usage. Introduce a boolean emit_frame_chain which
determines whether to store FP and LR and setup FP to point at this record.
When the frame pointer is enabled but not strictly required (eg. no use of
alloca), we emit a frame chain in non-leaf functions, but don't use the
frame pointer to access locals. This results in smaller code and unwind info.
Simplify the logic in aarch64_override_options_after_change_1 () and compute
whether the frame chain is required in aarch64_layout_frame () instead.
As a result aarch64_frame_pointer_required is now redundant.
Convert all callee save/restore functions to use gen_frame_mem.
Bootstrap OK.
ChangeLog:
2017-06-15 Wilco Dijkstra
gcc/
PR middle-end/60580
* config/aarch64/aarch64.h (aarch64_frame):
Add emit_frame_chain boolean.
* config/aarch64/aarch64.c (aarch64_frame_pointer_required)
Remove.
(aarch64_layout_frame): Initialise emit_frame_chain.
(aarch64_pushwb_single_reg): Use gen_frame_mem.
(aarch64_pop_regs): Likewise.
(aarch64_gen_load_pair): Likewise.
(aarch64_save_callee_saves): Likewise.
(aarch64_restore_callee_saves): Likewise.
(aarch64_expand_prologue): Use emit_frame_chain.
(aarch64_can_eliminate): Simplify. When FP needed or outgoing
arguments are large, eliminate to FP, otherwise SP.
(aarch64_override_options_after_change_1): Simplify.
(TARGET_FRAME_POINTER_REQUIRED): Remove define.
--
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index
08acdeb52d4083f50a4b44f43fb98009cdcc041f..722c39cfc4d57280d621fb6130e4d9f4d59d1e72
100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -591,6 +591,9 @@ struct GTY (()) aarch64_frame
/* The size of the stack adjustment after saving callee-saves. */
HOST_WIDE_INT final_adjust;
+ /* Store FP,LR and setup a frame pointer. */
+ bool emit_frame_chain;
+
unsigned wb_candidate1;
unsigned wb_candidate2;
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
fd3005d8056e65cb32c92bbd5eb752c977c885a5..a97b4bbe9dc0f7bccc90a9337519038041241531
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2761,24 +2761,6 @@ aarch64_output_probe_stack_range (rtx reg1, rtx reg2)
return "";
}
-static bool
-aarch64_frame_pointer_required (void)
-{
- /* In aarch64_override_options_after_change
- flag_omit_leaf_frame_pointer turns off the frame pointer by
- default. Turn it back on now if we've not got a leaf
- function. */
- if (flag_omit_leaf_frame_pointer
- && (!crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM)))
- return true;
-
- /* Force a frame pointer for EH returns so the return address is at FP+8. */
- if (crtl->calls_eh_return)
- return true;
-
- return false;
-}
-
/* Mark the registers that need to be saved by the callee and calculate
the size of the callee-saved registers area and frame record (both FP
and LR may be omitted). */
@@ -2791,6 +2773,18 @@ aarch64_layout_frame (void)
if (reload_completed && cfun->machine->frame.laid_out)
return;
+ /* Force a frame chain for EH returns so the return address is at FP+8. */
+ cfun->machine->frame.emit_frame_chain
+ = frame_pointer_needed || crtl->calls_eh_return;
+
+ /* Emit a frame chain if the frame pointer is enabled.
+ If -momit-leaf-frame-pointer is used, do not use a frame chain
+ in leaf functions which do not use LR. */
+ if (flag_omit_frame_pointer == 2
+ && !(flag_omit_leaf_frame_pointer && crtl->is_leaf
+ && !df_regs_ever_live_p (LR_REGNUM)))
+ cfun->machine->frame.emit_frame_chain = true;
+
#define SLOT_NOT_REQUIRED (-2)
#define SLOT_REQUIRED (-1)
@@ -2825,7 +2819,7 @@ aarch64_layout_frame (void)
last_fp_reg = regno;
}
- if (frame_pointer_needed)
+ if (cfun->machine->frame.emit_frame_chain)
{
/* FP and LR are placed in the linkage record. */
cfun->machine->frame.reg_offset[R29_REGNUM] = 0;
@@ -2997,7 +2991,7 @@ aarch64_pushwb_single_reg (machine_mode mode, unsigned
regno,
reg = gen_rtx_REG (mode, regno);
mem = gen_rtx_PRE_MODIFY (Pmode, base_rtx,
plus_constant (Pmode, base_rtx, -adjustment));
- mem = gen_rtx_MEM (mode, mem);
+ mem = gen_frame_mem (mode, mem);
insn = emit_move_insn (me
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 08/08
On 07/11/2017 11:21 PM, Jeff Law wrote: > This patch adds s390 support for stack-clash mitigation. > > s390's most interesting property is that the caller allocates space for > the callee to save registers into. > > So much like aarch64, we start with a very conservative assumption about > the offset between SP and the most recent stack probe. As we encounter > those register saves we may be able to decrease that offset. And like > aarch64 as we allocate space, the offset increases. If the offset > crosses PROBE_INTERVAL, we must emit probes. > > Because the register saves hit the caller's frame s390 in some ways > generates code more like x86/ppc. Though if there aren't any register > saves, then the resulting code looks more like aarch64. > > For large frames, I did not implement an allocate/probe in a loop. > Someone with a better understanding of the architecture is better suited > for that work. I'll note that you're going to need another scratch > register :-) This is the cause of the xfail of one test which expects > to see a prologue allocate/probe loop. > > s390 has a -mbackchain option. I'm not sure where it's used, but we do > try to handle it in the initial offset computation. However, we don't > handle it in the actual allocations that occur when -fstack-check=clash > is enabled. > > s390 does not have a -fstack-check=specific implementation. I have not > tried to add one. But I have defined STACK_CHECK_STATIC_BUILTIN. I > haven't investigated what side effects that might have. > > Other than the xfail noted above, the s390 uses the same tests as the > x86, ppc and aarch64 ports. > > I suspect we're going to need further iteration here. > > Thoughts/Comments? I'll have a look and run some tests to come back with an answer next week. Bye, -Andreas-
Re: [PATCH v3][AArch64] Fix symbol offset limit
ping
From: Wilco Dijkstra
Sent: 17 January 2017 15:14
To: Richard Earnshaw; GCC Patches; James Greenhalgh
Cc: nd
Subject: Re: [PATCH v3][AArch64] Fix symbol offset limit
Here is v3 of the patch - tree_fits_uhwi_p was necessary to ensure the size of a
declaration is an integer. So the question is whether we should allow
largish offsets outside of the bounds of symbols (v1), no offsets (this
version), or
small offsets (small negative and positive offsets just outside a symbol are
common).
The only thing we can't allow is any offset like we currently do...
In aarch64_classify_symbol symbols are allowed full-range offsets on
relocations.
This means the offset can use all of the +/-4GB offset, leaving no offset
available
for the symbol itself. This results in relocation overflow and link-time errors
for simple expressions like &global_char + 0xff00.
To avoid this, limit the offset to +/-1GB so that the symbol needs to be within
a
3GB offset from its references. For the tiny code model use a 64KB offset,
allowing
most of the 1MB range for code/data between the symbol and its references.
For symbols with a defined size, limit the offset to be within the size of the
symbol.
ChangeLog:
2017-01-17 Wilco Dijkstra
gcc/
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Apply reasonable limit to symbol offsets.
testsuite/
* gcc.target/aarch64/symbol-range.c (foo): Set new limit.
* gcc.target/aarch64/symbol-range-tiny.c (foo): Likewise.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
e8d65ead95a3c5730c2ffe64a9e057779819f7b4..f1d54e332dc1cf1ef0bc4b1e46b0ebebe1c4cea4
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9809,6 +9809,8 @@ aarch64_classify_symbol (rtx x, rtx offset)
if (aarch64_tls_symbol_p (x))
return aarch64_classify_tls_symbol (x);
+ const_tree decl = SYMBOL_REF_DECL (x);
+
switch (aarch64_cmodel)
{
case AARCH64_CMODEL_TINY:
@@ -9817,25 +9819,45 @@ aarch64_classify_symbol (rtx x, rtx offset)
we have no way of knowing the address of symbol at compile time
so we can't accurately say if the distance between the PC and
symbol + offset is outside the addressible range of +/-1M in the
- TINY code model. So we rely on images not being greater than
- 1M and cap the offset at 1M and anything beyond 1M will have to
- be loaded using an alternative mechanism. Furthermore if the
- symbol is a weak reference to something that isn't known to
- resolve to a symbol in this module, then force to memory. */
+ TINY code model. So we limit the maximum offset to +/-64KB and
+ assume the offset to the symbol is not larger than +/-(1M - 64KB).
+ Furthermore force to memory if the symbol is a weak reference to
+ something that doesn't resolve to a symbol in this module. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || INTVAL (offset) < -1048575 || INTVAL (offset) > 1048575)
+ || !IN_RANGE (INTVAL (offset), -0x1, 0x1))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_TINY_ABSOLUTE;
case AARCH64_CMODEL_SMALL:
/* Same reasoning as the tiny code model, but the offset cap here is
- 4G. */
+ 1G, allowing +/-3G for the offset to the symbol. */
if ((SYMBOL_REF_WEAK (x)
&& !aarch64_symbol_binds_local_p (x))
- || !IN_RANGE (INTVAL (offset), HOST_WIDE_INT_C (-4294967263),
- HOST_WIDE_INT_C (4294967264)))
+ || !IN_RANGE (INTVAL (offset), -0x4000, 0x4000))
return SYMBOL_FORCE_TO_MEM;
+
+ /* Limit offset to within the size of a declaration if available. */
+ if (decl && DECL_P (decl))
+ {
+ const_tree decl_size = DECL_SIZE (decl);
+
+ if (tree_fits_uhwi_p (decl_size)
+ && !IN_RANGE (INTVAL (offset), 0, tree_to_uhwi (decl_size)))
+ return SYMBOL_FORCE_TO_MEM;
+ }
+
return SYMBOL_SMALL_ABSOLUTE;
case AARCH64_CMODEL_TINY_PIC:
diff --git a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
b/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.c
index
d7e46b059e41f2672b3a1da5506fa8944e752e01..d49ff4dbe5786ef6d343d2b90052c09676dd7fe5
100644
--- a/gcc/testsuite/gcc.target/aarch64/symbol-range-tiny.
Re: [PATCH][AArch64] Improve aarch64_legitimate_constant_p
ping
This patch further improves aarch64_legitimate_constant_p. Allow all
integer, floating point and vector constants. Allow label references
and non-anchor symbols with an immediate offset. This allows such
constants to be rematerialized, resulting in smaller code and fewer stack
spills.
SPEC2006 codesize reduces by 0.08%, SPEC2017 by 0.13%.
Bootstrap OK, OK for commit?
ChangeLog:
2017-07-07 Wilco Dijkstra
* config/aarch64/aarch64.c (aarch64_legitimate_constant_p):
Return true for more constants, symbols and label references.
(aarch64_valid_floating_const): Remove unused function.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
a2eca64a9c13e44d223b5552c079ef4e09659e84..810c17416db01681e99a9eb8cc9f5af137ed2054
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10173,49 +10173,46 @@ aarch64_legitimate_pic_operand_p (rtx x)
return true;
}
-/* Return true if X holds either a quarter-precision or
- floating-point +0.0 constant. */
-static bool
-aarch64_valid_floating_const (machine_mode mode, rtx x)
-{
- if (!CONST_DOUBLE_P (x))
- return false;
-
- if (aarch64_float_const_zero_rtx_p (x))
- return true;
-
- /* We only handle moving 0.0 to a TFmode register. */
- if (!(mode == SFmode || mode == DFmode))
- return false;
-
- return aarch64_float_const_representable_p (x);
-}
+/* Implement TARGET_LEGITIMATE_CONSTANT_P hook. Return true for constants
+ that should be rematerialized rather than spilled. */
static bool
aarch64_legitimate_constant_p (machine_mode mode, rtx x)
{
+ /* Support CSE and rematerialization of common constants. */
+ if (CONST_INT_P (x) || CONST_DOUBLE_P (x) || GET_CODE (x) == CONST_VECTOR)
+ return true;
+
/* Do not allow vector struct mode constants. We could support
0 and -1 easily, but they need support in aarch64-simd.md. */
- if (TARGET_SIMD && aarch64_vect_struct_mode_p (mode))
+ if (aarch64_vect_struct_mode_p (mode))
return false;
- /* This could probably go away because
- we now decompose CONST_INTs according to expand_mov_immediate. */
- if ((GET_CODE (x) == CONST_VECTOR
- && aarch64_simd_valid_immediate (x, mode, false, NULL))
- || CONST_INT_P (x) || aarch64_valid_floating_const (mode, x))
- return !targetm.cannot_force_const_mem (mode, x);
+ /* Do not allow wide int constants - this requires support in movti. */
+ if (CONST_WIDE_INT_P (x))
+ return false;
- if (GET_CODE (x) == HIGH
- && aarch64_valid_symref (XEXP (x, 0), GET_MODE (XEXP (x, 0
- return true;
+ /* Do not allow const (plus (anchor_symbol, const_int)). */
+ if (GET_CODE (x) == CONST && GET_CODE (XEXP (x, 0)) == PLUS)
+ {
+ x = XEXP (XEXP (x, 0), 0);
+ if (SYMBOL_REF_P (x) && SYMBOL_REF_ANCHOR_P (x))
+ return false;
+ }
+
+ if (GET_CODE (x) == HIGH)
+ x = XEXP (x, 0);
/* Treat symbols as constants. Avoid TLS symbols as they are complex,
so spilling them is better than rematerialization. */
if (SYMBOL_REF_P (x) && !SYMBOL_REF_TLS_MODEL (x))
return true;
- return aarch64_constant_address_p (x);
+ /* Label references are always constant. */
+ if (GET_CODE (x) == LABEL_REF)
+ return true;
+
+ return false;
}
rtx
Re: [PATCH][AArch64] Fix PR79041
ping
As described in PR79041, -mcmodel=large -mpc-relative-literal-loads
may be used to avoid generating ADRP/ADD or ADRP/LDR. However both
trunk and GCC7 may still emit ADRP for some constant pool literals.
Fix this by adding a aarch64_pcrelative_literal_loads check.
OK for trunk/GCC7 backport?
ChangeLog:
2017-06-27 Wilco Dijkstra
PR target/79041
* config/aarch64/aarch64.c (aarch64_classify_symbol):
Avoid SYMBOL_SMALL_ABSOLUTE .
* testsuite/gcc.target/aarch64/pr79041-2.c: New test.
--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
060cd8476d2954119daac495ecb059c9be73edbe..329d244e9cf16dbdf849e5dd02b3999caf0cd5a7
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10042,7 +10042,7 @@ aarch64_classify_symbol (rtx x, rtx offset)
/* This is alright even in PIC code as the constant
pool reference is always PC relative and within
the same translation unit. */
- if (CONSTANT_POOL_ADDRESS_P (x))
+ if (CONSTANT_POOL_ADDRESS_P (x) && !aarch64_pcrelative_literal_loads)
return SYMBOL_SMALL_ABSOLUTE;
else
return SYMBOL_FORCE_TO_MEM;
diff --git a/gcc/testsuite/gcc.target/aarch64/pr79041-2.c
b/gcc/testsuite/gcc.target/aarch64/pr79041-2.c
new file mode 100644
index
..e7899725bad2b770f8488a07f99792113275bdf2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr79041-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mcmodel=large -mpc-relative-literal-loads" } */
+
+__int128
+t (void)
+{
+ return (__int128)1 << 80;
+}
+
+/* { dg-final { scan-assembler "adr" } } */
[patch,avr] Fix PR80929, work around middle-end PR81444
Hi, this patch restores some of the divmod handling.
It addresses two issues:
1) rtx_costs might be called with code = LSHIFTRT for a mul_highpart.
This is the case when outer_code = TRUNCATE. This patch uses a new
function to compute the costs for that case (also used for code = TRUNCATE).
2) Due to PR81444, the middle-end is calling the cost functions with
strange RTXes like
(truncate:HI (lshiftrt:PSI (mult:PSI (zero_extend:PSI (reg:TI 42))
(zero_extend:PSI (reg:TI 42)))
(const_int 16 [0x10])))
i.e. completely messed up modes. The correct RTX would be
(truncate:HI (lshiftrt:SI (mult:SI (zero_extend:SI (reg:HI 42))
(zero_extend:SI (reg:HI 42)))
(const_int 16 [0x10])))
The is worked around by also considering PSImode in the cost computation.
FYI, I am currently also just reg-testing a fix for PR81444, cf.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81444#c1
Ok to apply the patch below (trunk and v7)?
Johann
PR 80929
* config/avr/avr.c (avr_mul_highpart_cost): New static function.
(avr_rtx_costs_1) [TRUNCATE]: Use it to compute mul_highpart cost.
[LSHIFTRT, outer_code = TRUNCATE]: Same.
Index: config/avr/avr.c
===
--- config/avr/avr.c (revision 250156)
+++ config/avr/avr.c (working copy)
@@ -10709,6 +10709,33 @@ avr_memory_move_cost (machine_mode mode,
}
+/* Cost for mul highpart. X is a LSHIFTRT, i.e. the outer TRUNCATE is
+ already stripped off. */
+
+static int
+avr_mul_highpart_cost (rtx x, int)
+{
+ if (AVR_HAVE_MUL
+ && LSHIFTRT == GET_CODE (x)
+ && MULT == GET_CODE (XEXP (x, 0))
+ && CONST_INT_P (XEXP (x, 1)))
+{
+ // This is the wider mode.
+ machine_mode mode = GET_MODE (x);
+
+ // The middle-end might still have PR81444, i.e. it is calling the cost
+ // functions with strange modes. Fix this now by also considering
+ // PSImode (should actually be SImode instead).
+ if (HImode == mode || PSImode == mode || SImode == mode)
+{
+ return COSTS_N_INSNS (2);
+}
+}
+
+ return 1;
+}
+
+
/* Mutually recursive subroutine of avr_rtx_cost for calculating the
cost of an RTX operand given its context. X is the rtx of the
operand, MODE is its mode, and OUTER is the rtx_code of this
@@ -10748,7 +10775,7 @@ avr_operand_rtx_cost (rtx x, machine_mod
In either case, *TOTAL contains the cost result. */
static bool
-avr_rtx_costs_1 (rtx x, machine_mode mode, int outer_code ATTRIBUTE_UNUSED,
+avr_rtx_costs_1 (rtx x, machine_mode mode, int outer_code,
int opno ATTRIBUTE_UNUSED, int *total, bool speed)
{
enum rtx_code code = GET_CODE (x);
@@ -11402,6 +11429,12 @@ avr_rtx_costs_1 (rtx x, machine_mode mod
return true;
case LSHIFTRT:
+ if (outer_code == TRUNCATE)
+{
+ *total = avr_mul_highpart_cost (x, speed);
+ return true;
+}
+
switch (mode)
{
case QImode:
@@ -11579,16 +11612,10 @@ avr_rtx_costs_1 (rtx x, machine_mode mod
return true;
case TRUNCATE:
- if (AVR_HAVE_MUL
- && LSHIFTRT == GET_CODE (XEXP (x, 0))
- && MULT == GET_CODE (XEXP (XEXP (x, 0), 0))
- && CONST_INT_P (XEXP (XEXP (x, 0), 1)))
+ if (LSHIFTRT == GET_CODE (XEXP (x, 0)))
{
- if (QImode == mode || HImode == mode)
-{
- *total = COSTS_N_INSNS (2);
- return true;
-}
+ *total = avr_mul_highpart_cost (XEXP (x, 0), speed);
+ return true;
}
break;
[PATCH] allow deferred-shape pointers in OpenACC data clauses
This patch teaches the fortran FE to allow deferred-shape pointers to be
used in OpenACC data clauses. While the spec states that arrays must be
contiguous, I believe that is a run-time requirement, not a compile
time. The intent behind OpenACC is to have the programmer sprinkle a
minimum amount of ACC directives on their existing code base, and have
the compiler generate offloading code. In this case, the deferred-shape
pointer check was preventing working code from compiling.
I was considering relaxing the error to a warning, but I ended up
deciding this should be a run-time failure, if there is one.
Is this OK for trunk? I tested it on x86_64 with nvptx offloading.
Cesar
2017-07-14 Cesar Philippidis
gcc/fortran/
* openmp.c (check_array_not_assumed): Don't error on noncontiguous
deferred-shape pointers.
gcc/testsuite/
* gfortran.dg/goacc/deferred-shape-pointer.f90: New test.
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 8400354181c..f08a37e0d89 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -3774,10 +3774,6 @@ check_array_not_assumed (gfc_symbol *sym, locus loc, const char *name)
if (sym->as && sym->as->type == AS_ASSUMED_RANK)
gfc_error ("Assumed rank array %qs in %s clause at %L",
sym->name, name, &loc);
- if (sym->as && sym->as->type == AS_DEFERRED && sym->attr.pointer
- && !sym->attr.contiguous)
-gfc_error ("Noncontiguous deferred shape array %qs in %s clause at %L",
- sym->name, name, &loc);
}
static void
diff --git a/gcc/testsuite/gfortran.dg/goacc/deferred-shape-pointer.f90 b/gcc/testsuite/gfortran.dg/goacc/deferred-shape-pointer.f90
new file mode 100644
index 000..e0d0b8dd916
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/deferred-shape-pointer.f90
@@ -0,0 +1,20 @@
+! Test support of deferred-shape pointers.
+
+subroutine foo (x, y, z)
+ implicit none
+
+ integer :: y, z, i
+ integer,target :: x(y:z)
+ integer, pointer, dimension(:) :: px
+
+ px => x
+
+ !$acc data present(px)
+ !$acc end data
+
+ !$acc parallel loop present(px)
+ do i = 1, z
+ px(i) = px(i) * 2
+ end do
+ !$acc end parallel loop
+end subroutine foo
Re: C PATCH to display types when printing a conversion warning (PR c/81233)
On Fri, Jul 14, 2017 at 02:52:36PM +0200, Marek Polacek wrote: > On Thu, Jul 13, 2017 at 11:42:15AM -0600, Martin Sebor wrote: > > On 07/13/2017 08:18 AM, Marek Polacek wrote: > > > This patch improves diagnostic in the C FE by printing the types when > > > reporting > > > a problem with a conversion. E.g., instead of > > > > > >warning: assignment from incompatible pointer type > > > > > > you'll now get > > > > > > warning: assignment to 'int *' from incompatible pointer type 'char *' > > > > > > or instead of > > > > > > warning: initialization makes integer from pointer without a cast > > > > > > this > > > > > >warning: initialization of 'int *' from 'int' makes pointer from > > > integer without a cast > > > > > > I've been wanting this for a long time and here it is. Two snags: I had > > > to > > > make pedwarn_init to take '...' for which I had to introduce > > > emit_diagnostic_valist; you can't pass varargs from one vararg function to > > > another vararg function (and a macro with __VA_ARGS__ didn't work here). > > > Also, > > > PEDWARN_FOR_ASSIGNMENT didn't work with the addition of printing TYPE and > > > RHSTYPE so I just decided to unroll the macro instead of making it even > > > more > > > ugly. This patch is long but it's mainly because of the testsuite > > > fallout. > > > > > > If you have better ideas about the wording, let me know. > > > > It looks pretty good as is. My only wording suggestion is to > > consider simply mentioning conversion in the text of the warnings: > > > > warning: conversion to T* from an incompatible type U* > > > > I'm not sure that being explicit about the context where the bad > > conversion takes place (initialization vs assignment vs returning > > a value) is terribly helpful. That would not only simplify the > > code and make all the messages consistent, but it would also make > > it possible to get rid of the note when passing arguments. > > Yeah, I agree, actually. We print the expressions in question (although, > we could probably do even better), and I don't see why it would be > necessary to mention whether it's an initialization or an assignment. > I think I'll just drop this patch and do something along the lines you > suggest. > > David, do you agree with this? (Joseph's on PTO but I'd of course like > to hear his opinion, too.) I think I changed my mind. Because clang says e.g. warning: returning 'unsigned int *' from a function with result type 'int *' converts between pointers to integer types with different sign so in the end it might be best to just go with my current patch; it should only improve things anyway. And then add the fix-it hint. > One more thing: for > > int *q = p; > int i = q; > we should be able to provide a fix-it hint, something like 'did you mean > to dereference q?'. With the current PEDWARN_FOR_ASSIGNMENT macro it > would be awkward to implement that. > > > > There are still more warnings to improve but I think better to do this > > > incrementally rather than a single humongous patch. > > > > That makes sense. I was going to mention that it would be nice > > to also improve: > > > > warning: comparison of distinct pointer types lacks a cast > > > > If you take the conversion suggestion I think this warning would > > need to be phrased in terms of "conversion between T* and U*" > > rather than "conversion from T* to U*". (A similar change could > > be made to the error message printed when incompatible pointers > > are subtracted from one another.) > > Sure. > > Marek Marek
Re: [PATCH] Move static chain and non-local goto init after NOTE_INSN_FUNCTION_BEG (PR sanitize/81186).
Hi, On Thu, 13 Jul 2017, Martin Liška wrote: > Hopefully following patch will fix that. I returned to the first version > and saved/restored static_chain register before/after > __asan_stack_malloc. It should also work if you emit the parm_birth_note after the static chain is set up (not before it), but before you store into the nonlocal_goto_save_area. With that you don't need to worry about clobbering the incoming static chain with the asan setup. Can you test that? It would better reflect the intent of this note (the static chain being an implicit parameter, but the nonlocal_goto_save_area setup not being such). Ciao, Michael.
Re: C PATCH to fix bogus warning with -Wmultistatement-macros (PR c/81364)
I think David might be able to approve this one, so CCing.
On Tue, Jul 11, 2017 at 03:23:16PM +0200, Marek Polacek wrote:
> This patch fixes a bogus -Wmultistatement-macros warning. The code didn't
> notice that what came after a guard such as else was actually wrapped in { }
> which is a correct use. This bogus warning only triggered when the body of
> a conditional was coming from a different expansion than the conditional
> itself.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-07-11 Marek Polacek
>
> PR c/81364
> * c-parser.c (c_parser_else_body): Don't warn about multistatement
> macro expansion if the body is in { }.
> (c_parser_while_statement): Likewise.
> (c_parser_for_statement): Likewise.
>
> * Wmultistatement-macros-12.c: New test.
>
> diff --git gcc/c/c-parser.c gcc/c/c-parser.c
> index f8fbc92..7524a73 100644
> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -5557,7 +5557,8 @@ c_parser_else_body (c_parser *parser, const
> token_indent_info &else_tinfo,
> }
>else
> {
> - body_loc_after_labels = c_parser_peek_token (parser)->location;
> + if (!c_parser_next_token_is (parser, CPP_OPEN_BRACE))
> + body_loc_after_labels = c_parser_peek_token (parser)->location;
>c_parser_statement_after_labels (parser, NULL, chain);
> }
>
> @@ -5811,6 +5812,7 @@ c_parser_while_statement (c_parser *parser, bool ivdep,
> bool *if_p)
> = get_token_indent_info (c_parser_peek_token (parser));
>
>location_t loc_after_labels;
> + bool open_brace = c_parser_next_token_is (parser, CPP_OPEN_BRACE);
>body = c_parser_c99_block_statement (parser, if_p, &loc_after_labels);
>c_finish_loop (loc, cond, NULL, body, c_break_label, c_cont_label, true);
>add_stmt (c_end_compound_stmt (loc, block, flag_isoc99));
> @@ -5820,7 +5822,7 @@ c_parser_while_statement (c_parser *parser, bool ivdep,
> bool *if_p)
> = get_token_indent_info (c_parser_peek_token (parser));
>warn_for_misleading_indentation (while_tinfo, body_tinfo, next_tinfo);
>
> - if (next_tinfo.type != CPP_SEMICOLON)
> + if (next_tinfo.type != CPP_SEMICOLON && !open_brace)
> warn_for_multistatement_macros (loc_after_labels, next_tinfo.location,
> while_tinfo.location, RID_WHILE);
>
> @@ -6109,6 +6111,7 @@ c_parser_for_statement (c_parser *parser, bool ivdep,
> bool *if_p)
> = get_token_indent_info (c_parser_peek_token (parser));
>
>location_t loc_after_labels;
> + bool open_brace = c_parser_next_token_is (parser, CPP_OPEN_BRACE);
>body = c_parser_c99_block_statement (parser, if_p, &loc_after_labels);
>
>if (is_foreach_statement)
> @@ -6122,7 +6125,7 @@ c_parser_for_statement (c_parser *parser, bool ivdep,
> bool *if_p)
> = get_token_indent_info (c_parser_peek_token (parser));
>warn_for_misleading_indentation (for_tinfo, body_tinfo, next_tinfo);
>
> - if (next_tinfo.type != CPP_SEMICOLON)
> + if (next_tinfo.type != CPP_SEMICOLON && !open_brace)
> warn_for_multistatement_macros (loc_after_labels, next_tinfo.location,
> for_tinfo.location, RID_FOR);
>
> diff --git gcc/testsuite/c-c++-common/Wmultistatement-macros-12.c
> gcc/testsuite/c-c++-common/Wmultistatement-macros-12.c
> index e69de29..ac8915c 100644
> --- gcc/testsuite/c-c++-common/Wmultistatement-macros-12.c
> +++ gcc/testsuite/c-c++-common/Wmultistatement-macros-12.c
> @@ -0,0 +1,43 @@
> +/* PR c/81364 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wmultistatement-macros" } */
> +
> +#define FOO0 if (1) { } else
> +#define TST0 \
> +void bar0 (void) \
> +{ \
> + FOO0 { } /* { dg-bogus "macro expands to multiple statements" } */ \
> +}
> +TST0
> +
> +#define FOO1 for (;;)
> +#define TST1 \
> +void bar1 (void) \
> +{ \
> + FOO1 { } /* { dg-bogus "macro expands to multiple statements" } */ \
> +}
> +TST1
> +
> +#define FOO2 while (1)
> +#define TST2 \
> +void bar2 (void) \
> +{ \
> + FOO2 { } /* { dg-bogus "macro expands to multiple statements" } */ \
> +}
> +TST2
> +
> +#define FOO3 switch (1)
> +#define TST3 \
> +void bar3 (void) \
> +{ \
> + FOO3 { } /* { dg-bogus "macro expands to multiple statements" } */ \
> +}
> +TST3
> +
> +#define FOO4 if (1)
> +#define TST4 \
> +void bar4 (void) \
> +{ \
> + FOO4 { } /* { dg-bogus "macro expands to multiple statements" } */ \
> +}
> +TST4
>
> Marek
Marek
Re: c-family PATCH to improve -Wsign-compare (PR c/81417)
On Thu, Jul 13, 2017 at 04:59:20PM -0400, David Malcolm wrote:
> On Thu, 2017-07-13 at 16:39 -0400, Eric Gallager wrote:
> > On 7/13/17, David Malcolm wrote:
> > > On Thu, 2017-07-13 at 18:33 +0200, Marek Polacek wrote:
> > > > A tiny patch for -Wsign-compare so that is also prints the types
> > > > when
> > > > reporting a warning.
> > > >
> > > > Bootstrapped/regtested on x86_64-linux and ppc64le-redhat-linux,
> > > > ok
> > > > for trunk?
> > >
> > > Looks like it always display the types in the order signed then
> > > unsigned, which matches the text of the diagnostic, but not
> > > necessarily
> > > the ordering within the expression, which might be confusing if
> > > someone's comparing e.g.
> > >
> > > unsigned_a < signed_b
> > >
> >
> > Good catch, I forgot about that case when opening the original bug
> > that Marek posted this patch for...
> >
> > > But we already hardcode the ordering within the text of the
> > > diagnostic,
> > > so that feels excessively nit-picky.
> >
> > I don't think it's being excessively nit-picky; I think it'd make
> > more
> > sense to match the ordering of the expression. That's what clang
> > does:
> >
> > $ cat Wsign_compare.c
> > /* { dg-do compile } */
> >
> > int foo(signed int a, unsigned int b)
> > {
> > return (a < b);
> > }
> >
> > int bar(unsigned int c, signed int d)
> > {
> > return (c < d);
> > }
> >
> > $ /sw/opt/llvm-3.1/bin/clang -c -Wsign-compare Wsign_compare.c
> > Wsign_compare.c:5:12: warning: comparison of integers of different
> > signs: 'int' and 'unsigned int' [-Wsign-compare]
> > return (a < b);
> > ~ ^ ~
> > Wsign_compare.c:10:12: warning: comparison of integers of different
> > signs: 'unsigned int' and 'int' [-Wsign-compare]
> > return (c < d);
> > ~ ^ ~
> > 2 warnings generated.
>
> That's much nicer.
>
> >
> > >
> > > OK for trunk (with my "diagnostic messages" maintainer hat on).
>
> Marek: I take it back; can you update the patch accordingly, please?
>
> (Note to self: always doublecheck the linked PR for context).
Sure, here goes:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-07-14 Marek Polacek
PR c/81417
* c-warn.c (warn_for_sign_compare): Print the types.
* c-c++-common/Wsign-compare-1.c: New test.
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index b9378c2dbe2..7ff11821453 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -1891,9 +1891,10 @@ warn_for_sign_compare (location_t location,
c_common_signed_type (base_type)))
/* OK */;
else
- warning_at (location,
- OPT_Wsign_compare,
- "comparison between signed and unsigned integer
expressions");
+ warning_at (location, OPT_Wsign_compare,
+ "comparison between signed and unsigned integer "
+ "expressions: %qT and %qT", TREE_TYPE (orig_op0),
+ TREE_TYPE (orig_op1));
}
/* Warn if two unsigned values are being compared in a size larger
diff --git gcc/testsuite/c-c++-common/Wsign-compare-1.c
gcc/testsuite/c-c++-common/Wsign-compare-1.c
index e69de29bb2d..0e01453e7d8 100644
--- gcc/testsuite/c-c++-common/Wsign-compare-1.c
+++ gcc/testsuite/c-c++-common/Wsign-compare-1.c
@@ -0,0 +1,27 @@
+/* PR c/81417 */
+/* { dg-do compile } */
+/* { dg-options "-Wsign-compare" } */
+
+int
+fn1 (signed int a, unsigned int b)
+{
+ return a < b; /* { dg-warning "comparison between signed and unsigned
integer expressions: 'int' and 'unsigned int'" } */
+}
+
+int
+fn2 (signed int a, unsigned int b)
+{
+ return b < a; /* { dg-warning "comparison between signed and unsigned
integer expressions: 'unsigned int' and 'int'" } */
+}
+
+int
+fn3 (signed long int a, unsigned long int b)
+{
+ return b < a; /* { dg-warning "comparison between signed and unsigned
integer expressions: 'long unsigned int' and 'long int'" } */
+}
+
+int
+fn4 (signed short int a, unsigned int b)
+{
+ return b < a; /* { dg-warning "comparison between signed and unsigned
integer expressions: 'unsigned int' and 'short int'" } */
+}
Marek
Re: [PATCH 3/3, GCC/ARM] Add support for ARM Cortex-R52 processor
On 14/07/17 09:33, Thomas Preudhomme wrote: Hi, On 29/06/17 16:13, Thomas Preudhomme wrote: Please ignore this patch. I'll respin the patch on a more recent GCC. Please find an updated patch in attachment. This patch adds support for the ARM Cortex-R52 processor rencently announced. [1] https://developer.arm.com/products/processors/cortex-r/cortex-r52 ChangeLog entry is as follows: *** gcc/ChangeLog *** 2017-07-14 Thomas Preud'homme * config/arm/arm-cpus.in (cortex-r52): Add new entry. (armv8-r): Set ARM Cortex-R52 as default CPU. * config/arm/arm-tables.opt: Regenerate. * config/arm/arm-tune.md: Regenerate. * config/arm/driver-arm.c (arm_cpu_table): Add entry for ARM Cortex-R52. * doc/invoke.texi: Mention -mtune=cortex-r52 and availability of fp.dp extension for -mcpu=cortex-r52. Tested by building an arm-none-eabi GCC cross-compiler targeting Cortex-R52 and building an hello world with it. Also checked that the .fpu option created by GCC for -mcpu=cortex-r52 and -mcpu=cortex-r52+nofp.dp is as expected (respectively .fpu neon-fp-armv8 and .fpu fpv5-sp-d16 Is this ok for trunk? Ok. Thanks, Kyrill Best regards, Thomas
Re: C PATCH to display types when printing a conversion warning (PR c/81233)
On Thu, Jul 13, 2017 at 11:42:15AM -0600, Martin Sebor wrote: > On 07/13/2017 08:18 AM, Marek Polacek wrote: > > This patch improves diagnostic in the C FE by printing the types when > > reporting > > a problem with a conversion. E.g., instead of > > > >warning: assignment from incompatible pointer type > > > > you'll now get > > > > warning: assignment to 'int *' from incompatible pointer type 'char *' > > > > or instead of > > > > warning: initialization makes integer from pointer without a cast > > > > this > > > >warning: initialization of 'int *' from 'int' makes pointer from integer > > without a cast > > > > I've been wanting this for a long time and here it is. Two snags: I had to > > make pedwarn_init to take '...' for which I had to introduce > > emit_diagnostic_valist; you can't pass varargs from one vararg function to > > another vararg function (and a macro with __VA_ARGS__ didn't work here). > > Also, > > PEDWARN_FOR_ASSIGNMENT didn't work with the addition of printing TYPE and > > RHSTYPE so I just decided to unroll the macro instead of making it even more > > ugly. This patch is long but it's mainly because of the testsuite fallout. > > > > If you have better ideas about the wording, let me know. > > It looks pretty good as is. My only wording suggestion is to > consider simply mentioning conversion in the text of the warnings: > > warning: conversion to T* from an incompatible type U* > > I'm not sure that being explicit about the context where the bad > conversion takes place (initialization vs assignment vs returning > a value) is terribly helpful. That would not only simplify the > code and make all the messages consistent, but it would also make > it possible to get rid of the note when passing arguments. Yeah, I agree, actually. We print the expressions in question (although, we could probably do even better), and I don't see why it would be necessary to mention whether it's an initialization or an assignment. I think I'll just drop this patch and do something along the lines you suggest. David, do you agree with this? (Joseph's on PTO but I'd of course like to hear his opinion, too.) One more thing: for int *q = p; int i = q; we should be able to provide a fix-it hint, something like 'did you mean to dereference q?'. With the current PEDWARN_FOR_ASSIGNMENT macro it would be awkward to implement that. > > There are still more warnings to improve but I think better to do this > > incrementally rather than a single humongous patch. > > That makes sense. I was going to mention that it would be nice > to also improve: > > warning: comparison of distinct pointer types lacks a cast > > If you take the conversion suggestion I think this warning would > need to be phrased in terms of "conversion between T* and U*" > rather than "conversion from T* to U*". (A similar change could > be made to the error message printed when incompatible pointers > are subtracted from one another.) Sure. Marek
Re: Add support to trace comparison instructions and switch statements
On Thu, Jul 13, 2017 at 11:18 PM, Kostya Serebryany wrote:
>> > Hi
>> >
>> > I wrote a test for "-fsanitize-coverage=trace-cmp" .
>> >
>> > Is there anybody tells me if these codes could be merged into gcc ?
>>
>>
>> Nice!
>>
>> We are currently working on Linux kernel fuzzing that use the
>> comparison tracing. We use clang at the moment, but having this
>> support in gcc would be great for kernel land.
>>
>> One concern I have: do we want to do some final refinements to the API
>> before we implement this in both compilers?
>>
>> 2 things we considered from our perspective:
>> - communicating to the runtime which operands are constants
>> - communicating to the runtime which comparisons are counting loop checks
>>
>> First is useful if you do "find one operand in input and replace with
>> the other one" thing. Second is useful because counting loop checks
>> are usually not useful (at least all but one).
>> In the original Go implementation I also conveyed signedness of
>> operands, exact comparison operation (<, >, etc):
>> https://github.com/dvyukov/go-fuzz/blob/master/go-fuzz-defs/defs.go#L13
>> But I did not find any use for that.
>> I also gave all comparisons unique IDs:
>> https://github.com/dvyukov/go-fuzz/blob/master/go-fuzz-dep/sonar.go#L24
>> That turned out to be useful. And there are chances we will want this
>> for C/C++ as well.
>>
>> Kostya, did anything like this pop up in your work on libfuzzer?
>> Can we still change the clang API? At least add an additional argument
>> to the callbacks?
>
>
> I'd prefer not to change the API, but extend it (new compiler flag, new
> callbacks), if absolutely needed.
> Probably make it trace-cmp-guard (similar to trace-pc-guard, with an extra
> parameter that has the ID).
> I don't like the approach with compiler-generated constant IDs.
Yes, if we do it for C/C++, we need to create globals and pass pointer
to a global to the callbacks. IDs do not work for C/C++.
> Yes, it's a bit more efficient, but much less flexible in presence of
> multiple modules, DSOs, dlopen, etc.
>
> I was also looking at completely inlining this instrumentation because it's
> pretty expensive even in it's current form
> (adding more parameters will make things worse).
> This is going to be much less flexible, of course, so I'll attack it only
> once I settle on the algorithm to handle CMPs in libFuzzer.
This will require a new, completely different API for
compiler<->runtime anyway, so we can put this aside for now.
>> At the very least I would suggest that we add an additional arg that
>> contains some flags (1/2 arg is a const, this is counting loop check,
>> etc). If we do that we can also have just 1 callback that accepts
>> uint64's for args because we can pass operand size in the flags:
>
>
> How many flag combinations do we need and do we *really* need them?
>
> If the number of flag combinations is small, I'd prefer to have separate
> callbacks (__sanitizer_cov_trace_cmp_loop_bound ?)
>
> Do we really need to know that one arg is a const?
> It could well be a constant in fact, but compiler won't see it.
>
> int foo(int n) { ... if (i < n) ... }
> ...
> foo(42); // not inlined.
>
> We need to handle both cases the same way.
Well, following this line of reasoning we would need only
__asan_load/storeN callbacks for asan and remove
__asan_load/store1/2/4/8, because compiler might not know the size at
compile time. Constant-ness is only an optimization. If compiler does
not know that something is a const, fine. Based on my experience with
go-fuzz and our early experience with kernel, we badly need const
hint. Otherwise fuzzer generates gazillions of candidates based on
comparison arguments. Note that kernel is several order of magnitude
larger than what people usually fuzz in user-space, inputs are more
complex and at the same time execution speed is several order of
magnitude lower. We can't rely on raw speed.
Thinking of this more, I don't thing that globals will be useful in
the kernel context (the main problem is that we have multiple
transient isolated kernels). If we track per-comparison site
information, we will probably use PCs. So I am ready to give up on
this.
Both of you expressed concerns about performance. Kostya says we
should not break existing clang API.
If we limit this to only constant-ness, then I think we can make this
both forward and backward compatible, which means we don't need to
handle it now. E.g. we can:
- if both operands are const (if it's possible at all), don't emit any callback
- if only one is const, emit __sanitizer_cov_trace_cmp_const1 and
pass the const in a known position (i.e. always first/second arg)
- if none are const, emit callback __sanitizer_cov_trace_cmp_dyn1
Then compiler emits weak aliases form
__sanitizer_cov_trace_cmp_const/dyn1 to the old
__sanitizer_cov_trace_cmp1, which makes it backwards compatible.
New runtimes that implement __sanitizer_cov_trace_cmp_const/dyn1, also
need to provide the old __sanitizer
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 07/08
Jakub Jelinek wrote:
> On Wed, Jul 12, 2017 at 12:20:32AM +, Wilco Dijkstra wrote:
> > Therefore even when using a tiny 4K probe size we can safely adjust SP by
> > 3KB
> > before needing an explicit probe - now only 0.6% of functions need a probe.
> > If we choose a proper minimum probe distance, say 64KB, explicit probes are
> > basically non-existent (just 35 functions, or ~0.02% of all functions are >
> > 64KB).
> > Clearly inserting probes can be the default as the impact on code quality
> > is negligible.
>
> For non-leaf functions you need at least one probe no matter how small the
> frame size is (if it is bigger than 0), explicit or implicit, unless you
> perform IPA analysis on the callgraph and determine when that isn't needed,
> because you can have deep call stacks that would through functions that
> don't touch anything skip stack pages. Of course, such probes can be stores
> of call used registers, it can be any store to the stack.
Well you need to save the return address somewhere, so a non-leaf function
already
has an implicit probe before a call (even if shrinkwrapped). So it is not
possible for a
long sequence of function calls or a recursive function to jump the stack guard
- the
only way to jump the guard is using a huge unchecked static or dynamic
allocation.
One key thing to understand is that it doesn't matter where exactly the return
address
is saved in a frame. You could save it at a random location and all it would
mean is that
if the probe size is N, you only need to insert additional explicit probes if
the frame is
larger than N/2 (sum of static and dynamic allocation). Obviously you could do
better
than that with a well defined frame layout.
Before we consider IPA, how about optimizing trivial alloca's first? For
example why
does GCC emit dynamic allocations for:
void f(void*);
void alloca (int x)
{
if (x < 100)
f (__builtin_alloca (x));
f (__builtin_alloca (16));
}
Wilco
Re: [PATCH] i386: Avoid stack realignment if possible
On Fri, Jul 7, 2017 at 5:56 PM, H.J. Lu wrote:
> On Fri, Jul 07, 2017 at 09:58:42AM -0700, H.J. Lu wrote:
>> On Fri, Dec 20, 2013 at 8:06 AM, Jakub Jelinek wrote:
>> > Hi!
>> >
>> > Honza recently changed the i?86 backend, so that it often doesn't
>> > do -maccumulate-outgoing-args by default on x86_64.
>> > Unfortunately, on some of the here included testcases this regressed
>> > quite a bit the generated code. As AVX vectors are used, the dynamic
>> > realignment code needs to assume e.g. that some of them will need to be
>> > spilled, and for -mno-accumulate-outgoing-args the code needs to set
>> > need_drap early as well. But in when emitting the prologue/epilogue,
>> > if need_drap is set, we don't perform the optimization for leaf functions
>> > which have zero size stack frame, thus we end up with uselessly doing
>> > dynamic stack realignment, setting up DRAP that nothing uses and later on
>> > restore everything back.
>> >
>> > This patch improves it, if the DRAP register isn't live at the start of
>> > entry bb successor and we aren't going to realign the stack, we don't
>> > need DRAP at all, and even if we need DRAP register, that can't be the sole
>> > reason for doing stack realignment, the prologue code is able to set up
>> > DRAP
>> > even without dynamic stack realignment.
>> >
>> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>> >
>> > 2013-12-20 Jakub Jelinek
>> >
>> > PR target/59501
>> > * config/i386/i386.c (ix86_save_reg): Don't return true for
>> > drap_reg
>> > if !crtl->stack_realign_needed.
>> > (ix86_finalize_stack_realign_flags): If drap_reg isn't live on
>> > entry
>> > and stack_realign_needed will be false, clear drap_reg and
>> > need_drap.
>> > Optimize leaf functions that don't need stack frame even if
>> > crtl->need_drap.
>> >
>> > * gcc.target/i386/pr59501-1.c: New test.
>> > * gcc.target/i386/pr59501-1a.c: New test.
>> > * gcc.target/i386/pr59501-2.c: New test.
>> > * gcc.target/i386/pr59501-2a.c: New test.
>> > * gcc.target/i386/pr59501-3.c: New test.
>> > * gcc.target/i386/pr59501-3a.c: New test.
>> > * gcc.target/i386/pr59501-4.c: New test.
>> > * gcc.target/i386/pr59501-4a.c: New test.
>> > * gcc.target/i386/pr59501-5.c: New test.
>> > * gcc.target/i386/pr59501-6.c: New test.
>> >
>> >
>> > --- gcc/testsuite/gcc.target/i386/pr59501-4a.c.jj 2013-12-20
>> > 12:19:20.603212859 +0100
>> > +++ gcc/testsuite/gcc.target/i386/pr59501-4a.c 2013-12-20
>> > 12:23:33.647881672 +0100
>> > @@ -0,0 +1,8 @@
>> > +/* PR target/59501 */
>> > +/* { dg-do compile { target { ! ia32 } } } */
>> > +/* { dg-options "-O2 -mavx -maccumulate-outgoing-args" } */
>> > +
>> > +#include "pr59501-3a.c"
>> > +
>> > +/* Verify no dynamic realignment is performed. */
>> > +/* { dg-final { scan-assembler-not "and\[^\n\r]*sp" { xfail *-*-* } } } */
>> >
>>
>> Since DRAP isn't used with -maccumulate-outgoing-args, pr59501-4a.c was
>> xfailed due to stack frame access via frame pointer instead of DARP.
>> This patch finds the maximum stack alignment from the stack frame access
>> instructions and avoids stack realignment if stack alignment needed is
>> less than incoming stack boundary.
>>
>> I am testing this patch. OK for trunk if there is no regression?
>>
>>
>
> We need to keep the preferred stack alignment as the minimum stack
> alignment. Here is the updated patch. Tested on x86-64. OK for
> trunk?
>
> Thanks.
Hi Jakub,
This patch fixes the xfailed testcase in your patch:
https://gcc.gnu.org/ml/gcc-patches/2013-12/msg01767.html
Your original patch:
https://gcc.gnu.org/ml/gcc-patches/2011-11/msg01058.html
assumes that any instructions accessing stack require stack
realignment:
+ FOR_EACH_BB (bb)
+{
+ rtx insn;
+ FOR_BB_INSNS (bb, insn)
+if (NONDEBUG_INSN_P (insn)
+ && requires_stack_frame_p (insn, prologue_used,
+ set_up_by_prologue))
+ {
+ crtl->stack_realign_needed = stack_realign;
+ crtl->stack_realign_finalized = true;
+ return;
+ }
+ }
This patch checks the actual alignment needed for any instructions
accessing stack. It skips stack realignment if the actual stack alignment
needed is less than or equal to incoming stack alignment.
Can you take look at it?
Thanks.
> H.J.
> ---
> Since DRAP isn't used with -maccumulate-outgoing-args, pr59501-4a.c was
> xfailed due to stack frame access via frame pointer instead of DARP.
> This patch finds the maximum stack alignment from the stack frame access
> instructions and avoids stack realignment if stack alignment needed is
> less than incoming stack boundary.
>
> gcc/
>
> PR target/59501
> * config/i386/i386.c (ix86_finalize_stack_realign_flags): Don't
> realign stack if stack alignment needed is less than incoming
> stack boundary.
>
> gcc/testsuite/
>
> PR target/59501
>
[PATCH] [SPARC/RTEMS]: Add multilibs for LEON3FT back-to-back store workaround
Replace MULTILIB_EXCEPTIONS with MULTILIB_REQUIRED for readability. -mfix-gr712rc and -mfix-ut700 are currently equivalent. 2017-07-14 Daniel Cederman * config/sparc/t-rtems: Add mfix-gr712rc multilibs. Replace MULTILIB_EXCEPTIONS with MULTILIB_REQUIRED. Match -mfix-gr712rc with -mfix-ut700. --- gcc/config/sparc/t-rtems | 36 +--- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/gcc/config/sparc/t-rtems b/gcc/config/sparc/t-rtems index 7411aaa..809840c 100644 --- a/gcc/config/sparc/t-rtems +++ b/gcc/config/sparc/t-rtems @@ -18,19 +18,25 @@ # MULTILIB_OPTIONS = msoft-float mcpu=v8/mcpu=leon3/mcpu=leon3v7/mcpu=leon \ - mfix-ut699/mfix-at697f -MULTILIB_DIRNAMES = soft v8 leon3 leon3v7 leon ut699 at697f -MULTILIB_MATCHES = msoft-float=mno-fpu + mfix-ut699/mfix-at697f/mfix-gr712rc +MULTILIB_DIRNAMES = soft v8 leon3 leon3v7 leon ut699 at697f gr712rc +MULTILIB_MATCHES = msoft-float=mno-fpu mfix-gr712rc=mfix-ut700 -MULTILIB_EXCEPTIONS = mfix-ut699 -MULTILIB_EXCEPTIONS += msoft-float/mfix-ut699 -MULTILIB_EXCEPTIONS += msoft-float/mcpu=v8/mfix-ut699 -MULTILIB_EXCEPTIONS += msoft-float/mcpu=leon3*/mfix-ut699 -MULTILIB_EXCEPTIONS += mcpu=v8/mfix-ut699 -MULTILIB_EXCEPTIONS += mcpu=leon3*/mfix-ut699 -MULTILIB_EXCEPTIONS += mfix-at697f -MULTILIB_EXCEPTIONS += msoft-float/mfix-at697f -MULTILIB_EXCEPTIONS += msoft-float/mcpu=v8/mfix-at697f -MULTILIB_EXCEPTIONS += msoft-float/mcpu=leon3*/mfix-at697f -MULTILIB_EXCEPTIONS += mcpu=v8/mfix-at697f -MULTILIB_EXCEPTIONS += mcpu=leon3*/mfix-at697f +MULTILIB_REQUIRED = +MULTILIB_REQUIRED += msoft-float +MULTILIB_REQUIRED += mcpu=v8 +MULTILIB_REQUIRED += mcpu=leon3 +MULTILIB_REQUIRED += mcpu=leon3v7 +MULTILIB_REQUIRED += mcpu=leon +MULTILIB_REQUIRED += mcpu=leon3/mfix-gr712rc +MULTILIB_REQUIRED += mcpu=leon3v7/mfix-gr712rc +MULTILIB_REQUIRED += mcpu=leon/mfix-ut699 +MULTILIB_REQUIRED += mcpu=leon/mfix-at697f +MULTILIB_REQUIRED += msoft-float/mcpu=v8 +MULTILIB_REQUIRED += msoft-float/mcpu=leon3 +MULTILIB_REQUIRED += msoft-float/mcpu=leon3v7 +MULTILIB_REQUIRED += msoft-float/mcpu=leon +MULTILIB_REQUIRED += msoft-float/mcpu=leon3/mfix-gr712rc +MULTILIB_REQUIRED += msoft-float/mcpu=leon3v7/mfix-gr712rc +MULTILIB_REQUIRED += msoft-float/mcpu=leon/mfix-ut699 +MULTILIB_REQUIRED += msoft-float/mcpu=leon/mfix-at697f -- 2.9.3
Re: [PATCH AArch64]Fix ICE in cortex-a57 fma steering pass
On Wed, Jul 12, 2017 at 03:15:04PM +, Bin Cheng wrote:
> Hi,
> After change @236817, AArch64 backend could avoid unnecessary conversion
> instructions for register between different modes now. As a result, GCC
> could initialize register in larger mode and use it later in smaller mode.
> such def-use chain is not supported by current regrename.c analyzer, as
> described by its comment:
>
> /* Process the insn, determining its effect on the def-use
>chains and live hard registers. We perform the following
>steps with the register references in the insn, simulating
>its effect:
> ...
>We cannot deal with situations where we track a reg in one mode
>and see a reference in another mode; these will cause the chain
>to be marked unrenamable or even cause us to abort the entire
>basic block. */
>
> In this case, regrename.c analyzer doesn't create chain for the use of the
> register. OTOH, cortex-a57-fma-steering.c has below code:
>
> @@ -973,10 +973,14 @@ func_fma_steering::analyze ()
> break;
> }
>
> - /* We didn't find a chain with a def for this instruction. */
> - gcc_assert (i < dest_op_info->n_chains);
> -
> - this->analyze_fma_fmul_insn (forest, chain, head);
>
> It assumes by gcc_assert that a chain must be found for dest register of
> fmul/fmac instructions. According to above analysis, this is not always true
> if the dest reg is reused from one of its source register.
>
> This patch fixes the issue by skipping such instructions if no du chain is
> found. Bootstrap and test on AArch64/cortex-a57. Is it OK? If it's fine, I
> would also need to backport it to 7/6 branches.
This looks OK, but feels a bit like a workaround. Do you have a PR open
for the missed optimisation caused by the deficiency in regrename?
If so, it would be good to add that PR number to your comment in this
function.
For now, and for the backport, this will be fine, but your (Kyrill's) testcase
has confused me (maybe too reduced from the original form) and doesn't
match the bug here.
> 2017-07-12 Bin Cheng
>
> PR target/81414
> * config/aarch64/cortex-a57-fma-steering.c (analyze): Skip fmul/fmac
> instructions if no du chain is found.
>
> gcc/testsuite/ChangeLog
> 2017-07-12 Kyrylo Tkachov
>
> PR target/81414
> * gcc.target/aarch64/pr81414.C: New.
> From ef2bc842993210a4399205d83fa46435eec5d7cd Mon Sep 17 00:00:00 2001
> From: Bin Cheng
> Date: Wed, 12 Jul 2017 15:16:53 +0100
> Subject: [PATCH] tmp
>
> ---
> gcc/config/aarch64/cortex-a57-fma-steering.c | 12
> gcc/testsuite/gcc.target/aarch64/pr81414.C | 10 ++
> 2 files changed, 18 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr81414.C
>
> diff --git a/gcc/config/aarch64/cortex-a57-fma-steering.c
> b/gcc/config/aarch64/cortex-a57-fma-steering.c
> index 1bf804b..b2ee398 100644
> --- a/gcc/config/aarch64/cortex-a57-fma-steering.c
> +++ b/gcc/config/aarch64/cortex-a57-fma-steering.c
> @@ -973,10 +973,14 @@ func_fma_steering::analyze ()
> break;
> }
>
> - /* We didn't find a chain with a def for this instruction. */
> - gcc_assert (i < dest_op_info->n_chains);
> -
> - this->analyze_fma_fmul_insn (forest, chain, head);
> + /* Due to implementation of regrename, dest register can slip away
> + from regrename's analysis. As a result, there is no chain for
> + the destination register of insn. We simply skip the insn even
> + it is a fmul/fmac instruction. This case can happen when the
> + dest register is also a source register of insn and the source
> + reg is setup in larger mode than this insn. */
> + if (i < dest_op_info->n_chains)
> + this->analyze_fma_fmul_insn (forest, chain, head);
> }
> }
>free (bb_dfs_preorder);
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr81414.C
> b/gcc/testsuite/gcc.target/aarch64/pr81414.C
> new file mode 100644
> index 000..13666a3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr81414.C
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mcpu=cortex-a57" } */
> +
> +typedef __Float32x2_t float32x2_t;
> +__inline float32x2_t vdup_n_f32(float) {}
> +
> +float32x2_t vfma_lane_f32(float32x2_t __a, float32x2_t __b) {
> + int __lane;
> + return __builtin_aarch64_fmav2sf(__b, vdup_n_f32(__lane), __a);
> +}
I don't see a mode-change here. This looks like it would have a bad def/use
chain because of the unitialised __lane, rather than the issue here.
In practice, your patch probably fixes two related issues - one where the
def/use chain can't be formed because the register is used unitialised
(this testcase) and one where the def/use chain can't be tracked through a
mode-change.
While fixing both of these in one go i
[PATCH] [SPARC/RTEMS]: Add __FIX_LEON3FT_B2BST
In case the LEON3FT back-to-back store workaround is active
(sparc_fix_b2bst), then define the builtin define __FIX_LEON3FT_B2BST on
RTEMS. The intended use case for this is operating system code in
assembly language. See also:
https://lists.rtems.org/pipermail/devel/2017-July/018463.html
gcc/
* gcc/config/sparc/rtemself.h (TARGET_OS_CPP_BUILTINS): Add
conditional builtin define __FIX_LEON3FT_B2BST.
---
gcc/config/sparc/rtemself.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gcc/config/sparc/rtemself.h b/gcc/config/sparc/rtemself.h
index e5a3f6eae03..10eaa08fe4e 100644
--- a/gcc/config/sparc/rtemself.h
+++ b/gcc/config/sparc/rtemself.h
@@ -26,6 +26,8 @@ along with GCC; see the file COPYING3. If not see
builtin_define ("__rtems__"); \
builtin_define ("__USE_INIT_FINI__"); \
builtin_assert ("system=rtems");\
+ if (sparc_fix_b2bst)\
+ builtin_define ("__FIX_LEON3FT_B2BST"); \
} \
while (0)
--
2.12.3
Re: [Patch][Aarch64] Refactor comments in aarch64_print_operand
On Fri, Jul 14, 2017 at 10:11:51AM +0100, Jackson Woodruff wrote: > Hi James, > > I have addressed the issues below. > > OK for trunk? This looks good to me, thanks for the patch. I've committed this to trunk on your behalf as revision 250202. Thanks, James
[committed] Fix libsanitizer build against recent glibc (PR sanitizer/81066)
Hi!
glibc recently changed a couple of headers. One change was
that typedef struct sigaltstack { ... } stack_t; is now
typedef struct { ... } stack_t;
and the other change is that resolv.h now ignores the former (private glibc
macro) __need_res_state macro and after
struct __res_state { ... };
typedef struct __res_state *res_state;
also newly adds
extern struct __res_state *__res_state(void) __attribute__ ((__const__));
declaration even if that macro is defined.
The following patch fixes both of these issues and should work even with
older glibcs.
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
Will backport to release branches soon.
2017-07-14 Jakub Jelinek
PR sanitizer/81066
* sanitizer_common/sanitizer_linux.h: Cherry-pick upstream r307969.
* sanitizer_common/sanitizer_linux.cc: Likewise.
* sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc: Likewise.
* tsan/tsan_platform_linux.cc: Likewise.
--- libsanitizer/sanitizer_common/sanitizer_linux.h (revision 307968)
+++ libsanitizer/sanitizer_common/sanitizer_linux.h (revision 307969)
@@ -21,7 +21,6 @@
#include "sanitizer_platform_limits_posix.h"
struct link_map; // Opaque type returned by dlopen().
-struct sigaltstack;
namespace __sanitizer {
// Dirent structure for getdents(). Note that this structure is different from
@@ -30,8 +29,7 @@ struct linux_dirent;
// Syscall wrappers.
uptr internal_getdents(fd_t fd, struct linux_dirent *dirp, unsigned int count);
-uptr internal_sigaltstack(const struct sigaltstack* ss,
- struct sigaltstack* oss);
+uptr internal_sigaltstack(const void* ss, void* oss);
uptr internal_sigprocmask(int how, __sanitizer_sigset_t *set,
__sanitizer_sigset_t *oldset);
--- libsanitizer/sanitizer_common/sanitizer_linux.cc(revision 307968)
+++ libsanitizer/sanitizer_common/sanitizer_linux.cc(revision 307969)
@@ -629,8 +629,7 @@ uptr internal_prctl(int option, uptr arg
}
#endif
-uptr internal_sigaltstack(const struct sigaltstack *ss,
- struct sigaltstack *oss) {
+uptr internal_sigaltstack(const void *ss, void *oss) {
return internal_syscall(SYSCALL(sigaltstack), (uptr)ss, (uptr)oss);
}
--- libsanitizer/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
(revision 307968)
+++ libsanitizer/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
(revision 307969)
@@ -287,7 +287,7 @@ static int TracerThread(void* argument)
// Alternate stack for signal handling.
InternalScopedBuffer handler_stack_memory(kHandlerStackSize);
- struct sigaltstack handler_stack;
+ stack_t handler_stack;
internal_memset(&handler_stack, 0, sizeof(handler_stack));
handler_stack.ss_sp = handler_stack_memory.data();
handler_stack.ss_size = kHandlerStackSize;
--- libsanitizer/tsan/tsan_platform_linux.cc(revision 307968)
+++ libsanitizer/tsan/tsan_platform_linux.cc(revision 307969)
@@ -286,7 +286,7 @@ void InitializePlatform() {
int ExtractResolvFDs(void *state, int *fds, int nfd) {
#if SANITIZER_LINUX && !SANITIZER_ANDROID
int cnt = 0;
- __res_state *statp = (__res_state*)state;
+ struct __res_state *statp = (struct __res_state*)state;
for (int i = 0; i < MAXNS && cnt < nfd; i++) {
if (statp->_u._ext.nsaddrs[i] && statp->_u._ext.nssocks[i] != -1)
fds[cnt++] = statp->_u._ext.nssocks[i];
Jakub
Re: [PATCH] Improve bswap on nop non-base_addr reshuffles (PR tree-optimization/81396)
On 14/07/17 09:56, Jakub Jelinek wrote:
On Fri, Jul 14, 2017 at 09:45:39AM +0100, Thomas Preudhomme wrote:
+ else if (!bswap)
+{
Would it make sense to add an assert right here checking that this is a
cmpnop operation?
The earlier bswap_replace code to handle n->base_addr && !bswap doesn't
have anything like that either.
And the assert would be certainly non-trivial, like:
#if ENABLE_ASSERT_CHECKING
uint64_t cmpnop = CMPNOP;
/* Note, n->range has been multiplied by BITS_PER_UNIT in between, so it
needs to be divided by it again. */
if (n->range / BITS_PER_UNIT < (int) sizeof (int64_t))
{
mask = ((uint64_t) 1 << (n->range / BITS_PER_UNIT * BITS_PER_MARKER)) -
1;
cmpnop &= mask;
}
gcc_assert (n->n == cmpnop);
#endif
I think trusting find_bswap_or_nop did the job right is enough.
Ooops I meant check that if !base_addr then !bswap but that's already guaranteed
by the structure of the if. All good then.
Best regards,
Thomas
Re: [Patch][Aarch64] Refactor comments in aarch64_print_operand
Hi James,
I have addressed the issues below.
OK for trunk?
Jackson
On 07/13/2017 05:14 PM, James Greenhalgh wrote:
On Thu, Jul 13, 2017 at 04:35:55PM +0100, Jackson Woodruff wrote:
Hi James,
I've addressed the issues discussed below.
OK for trunk?
I one last comment, otherwise, this looks good:
+/* Print operand X to file F in a target specific manner according to CODE.
+ The acceptable formatting commands given by CODE are:
+ 'c': An integer or symbol address without a preceding # sign.
+ 'e': Print the sign/zero-extend size as a character 8->b,
+ 16->h, 32->w.
+ 'p': Prints N such that 2^N == X (X must be power of 2 and
+ const int).
+ 'P': Print the number of non-zero bits in X (a const_int).
+ 'H': Print the higher numbered register of a pair (TImode)
+ of regs.
+ 'm': Print a condition (eq, ne, etc).
+ 'M': Same as 'm', but invert condition.
+ 'b/h/s/d/q': Print a scalar FP/SIMD register name.
+ 'S/T/U/V':Print the first FP/SIMD register name in a list
+ (No difference between any of these options).
There is a slight difference between these options - You'd use them in a
in a pattern with a large integer mode like LD3 on a CImode value to print
the register list you want to load. For example:
LD3 {v0.4s - v2.4s} [x0]
The register number you'll get by inspecting REGNO (x) will give you the
start of the register list - but we need to get the right number for the
end of the register list too. To find that offset, we take
(CODE - 'S'). It should be clear why for S/T/U/V this gives 0/1/2/3.
So this comment should read:
Print a FP/SIMD register name for a register list. The register
printed is the FP/SIMD register name of X + 0/1/2/3 for S/T/U/V.
Or similar.
Thanks,
James
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index
037339d431d80c49699446e548d6b2707883b6a8..e77054c5e23aa876add1629d9eb13347441718cb
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5053,12 +5053,42 @@ static const int aarch64_nzcv_codes[] =
0/* NV, Any. */
};
+/* Print operand X to file F in a target specific manner according to CODE.
+ The acceptable formatting commands given by CODE are:
+ 'c': An integer or symbol address without a preceding # sign.
+ 'e': Print the sign/zero-extend size as a character 8->b,
+ 16->h, 32->w.
+ 'p': Prints N such that 2^N == X (X must be power of 2 and
+ const int).
+ 'P': Print the number of non-zero bits in X (a const_int).
+ 'H': Print the higher numbered register of a pair (TImode)
+ of regs.
+ 'm': Print a condition (eq, ne, etc).
+ 'M': Same as 'm', but invert condition.
+ 'b/h/s/d/q': Print a scalar FP/SIMD register name.
+ 'S/T/U/V':Print a FP/SIMD register name for a register
list.
+ The register printed is the FP/SIMD register name
+ of X + 0/1/2/3 for S/T/U/V.
+ 'R': Print a scalar FP/SIMD register name + 1.
+ 'X': Print bottom 16 bits of integer constant in hex.
+ 'w/x':Print a general register name or the zero register
+ (32-bit or 64-bit).
+ '0': Print a normal operand, if it's a general register,
+ then we assume DImode.
+ 'k': Print NZCV for conditional compare instructions.
+ 'A': Output address constant representing the first
+ argument of X, specifying a relocation offset
+ if appropriate.
+ 'L': Output constant address specified by X
+ with a relocation offset if appropriate.
+ 'G': Prints address of X, specifying a PC relative
+ relocation mode if appropriate. */
+
static void
aarch64_print_operand (FILE *f, rtx x, int code)
{
switch (code)
{
-/* An integer or symbol address without a preceding # sign. */
case 'c':
switch (GET_CODE (x))
{
@@ -5085,7 +5115,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
break;
case 'e':
- /* Print the sign/zero-extend size as a character 8->b, 16->h, 32->w. */
{
int n;
@@ -5118,7 +5147,6 @@ aarch64_print_operand (FILE *f, rtx x, int code)
{
int n;
- /* Print N such that 2^N == X. */
if (!CONST_INT_P (x) || (n = exact_log2 (INTVAL (x))) < 0)
{
output_operand_lossage ("invalid operand for '%%%c'", code);
@@ -5130,7 +5158,6 @@ aarch64_print_operand (FILE *f, r
[PATCH][committed] Update comment in gimple-ssa-store-merging.c
Hi all,
Jakub replaced the simple loop zeroing parts of the array in a previous patch.
This patchlet just adjusts the comment to be a bit more accurate.
Committed as obvious.
Thanks,
Kyrill
2017-07-14 Kyrylo Tkachov
* gimple-ssa-store-merging.c (clear_bit_region): Replace reference to
loop in comment with memset.
diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c
index 17a95a50b1fee8ae989343fe63955ab5c045f43b..05b8cd9516a3f863a0f8ce598a69ab8f52a1c441 100644
--- a/gcc/gimple-ssa-store-merging.c
+++ b/gcc/gimple-ssa-store-merging.c
@@ -331,8 +331,8 @@ clear_bit_region (unsigned char *ptr, unsigned int start,
else if (start == 0 && len > BITS_PER_UNIT)
{
unsigned int nbytes = len / BITS_PER_UNIT;
- /* We could recurse on each byte but do the loop here to avoid
- recursing too deep. */
+ /* We could recurse on each byte but we clear whole bytes, so a simple
+ memset will do. */
memset (ptr, '\0', nbytes);
/* Clear the remaining sub-byte region if there is one. */
if (len % BITS_PER_UNIT != 0)
Re: [PATCH] Improve bswap on nop non-base_addr reshuffles (PR tree-optimization/81396)
On Fri, Jul 14, 2017 at 09:45:39AM +0100, Thomas Preudhomme wrote:
> > + else if (!bswap)
> > +{
>
> Would it make sense to add an assert right here checking that this is a
> cmpnop operation?
The earlier bswap_replace code to handle n->base_addr && !bswap doesn't
have anything like that either.
And the assert would be certainly non-trivial, like:
#if ENABLE_ASSERT_CHECKING
uint64_t cmpnop = CMPNOP;
/* Note, n->range has been multiplied by BITS_PER_UNIT in between, so it
needs to be divided by it again. */
if (n->range / BITS_PER_UNIT < (int) sizeof (int64_t))
{
mask = ((uint64_t) 1 << (n->range / BITS_PER_UNIT * BITS_PER_MARKER)) - 1;
cmpnop &= mask;
}
gcc_assert (n->n == cmpnop);
#endif
I think trusting find_bswap_or_nop did the job right is enough.
Jakub
Re: [PATCH] Improve bswap on nop non-base_addr reshuffles (PR tree-optimization/81396)
Hi Jakub,
On 13/07/17 21:51, Jakub Jelinek wrote:
Hi!
As mentioned in the PR, the following testcase started using recently
BIT_FIELD_REFs instead of MEM_REFs and thus the bswap pass, while it
properly determines the very long sequence of stmts is a nop transformation,
throws that away and doesn't optimize it, and no other optimizations
are able to optimize it away.
The patch attempts to not do anything if there is a simple identity
copy, but if the nop reshuffle needs more than one operation, it will
try to replace the final SSA_NAME BIT_IOR_EXPR assignment with assignment
from the src value (typically SSA_NAME).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-07-13 Jakub Jelinek
PR tree-optimization/81396
* tree-ssa-math-opts.c (struct symbolic_number): Add n_ops field.
(init_symbolic_number): Initialize it to 1.
(perform_symbolic_merge): Add n_ops from both operands into the new
n_ops.
(find_bswap_or_nop): Don't consider n->n == cmpnop computations
without base_addr as useless if they need more than one operation.
(bswap_replace): Handle !bswap case for NULL base_addr.
* gcc.dg/tree-ssa/pr81396.c: New test.
--- gcc/tree-ssa-math-opts.c.jj 2017-07-06 20:31:43.0 +0200
+++ gcc/tree-ssa-math-opts.c2017-07-13 19:27:02.985354778 +0200
@@ -1968,6 +1968,7 @@ struct symbolic_number {
tree alias_set;
tree vuse;
unsigned HOST_WIDE_INT range;
+ int n_ops;
};
#define BITS_PER_MARKER 8
@@ -2083,6 +2084,7 @@ init_symbolic_number (struct symbolic_nu
return false;
n->range = size;
n->n = CMPNOP;
+ n->n_ops = 1;
if (size < 64 / BITS_PER_MARKER)
n->n &= ((uint64_t) 1 << (size * BITS_PER_MARKER)) - 1;
@@ -2293,6 +2295,7 @@ perform_symbolic_merge (gimple *source_s
return NULL;
}
n->n = n1->n | n2->n;
+ n->n_ops = n1->n_ops + n2->n_ops;
return source_stmt;
}
@@ -2588,7 +2591,7 @@ find_bswap_or_nop (gimple *stmt, struct
return NULL;
/* Useless bit manipulation performed by code. */
- if (!n->base_addr && n->n == cmpnop)
+ if (!n->base_addr && n->n == cmpnop && n->n_ops == 1)
return NULL;
n->range *= BITS_PER_UNIT;
@@ -2747,6 +2750,36 @@ bswap_replace (gimple *cur_stmt, gimple
}
src = val_tmp;
}
+ else if (!bswap)
+{
Would it make sense to add an assert right here checking that this is a cmpnop
operation?
Looks good to me otherwise.
Best regards,
Thomas
+ gimple *g;
+ if (!useless_type_conversion_p (TREE_TYPE (tgt), TREE_TYPE (src)))
+ {
+ if (!is_gimple_val (src))
+ return false;
+ g = gimple_build_assign (tgt, NOP_EXPR, src);
+ }
+ else
+ g = gimple_build_assign (tgt, src);
+ if (n->range == 16)
+ nop_stats.found_16bit++;
+ else if (n->range == 32)
+ nop_stats.found_32bit++;
+ else
+ {
+ gcc_assert (n->range == 64);
+ nop_stats.found_64bit++;
+ }
+ if (dump_file)
+ {
+ fprintf (dump_file,
+ "%d bit reshuffle in target endianness found at: ",
+ (int) n->range);
+ print_gimple_stmt (dump_file, cur_stmt, 0);
+ }
+ gsi_replace (&gsi, g, true);
+ return true;
+}
else if (TREE_CODE (src) == BIT_FIELD_REF)
src = TREE_OPERAND (src, 0);
--- gcc/testsuite/gcc.dg/tree-ssa/pr81396.c.jj 2017-07-13 19:22:10.191954620 +0200
+++ gcc/testsuite/gcc.dg/tree-ssa/pr81396.c 2017-07-13 19:24:16.638399984
+0200
@@ -0,0 +1,25 @@
+/* PR tree-optimization/81396 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+typedef unsigned long long uint64_t;
+
+uint64_t
+foo (uint64_t word)
+{
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ && __SIZEOF_LONG_LONG__ == 8
+ const unsigned char *const ptr = (const unsigned char *) &word;
+ return ((uint64_t) ptr[0]
+ | ((uint64_t) ptr[1] << 8)
+ | ((uint64_t) ptr[2] << (8 * 2))
+ | ((uint64_t) ptr[3] << (8 * 3))
+ | ((uint64_t) ptr[4] << (8 * 4))
+ | ((uint64_t) ptr[5] << (8 * 5))
+ | ((uint64_t) ptr[6] << (8 * 6))
+ | ((uint64_t) ptr[7] << (8 * 7)));
+#else
+ return word;
+#endif
+}
+
+/* { dg-final { scan-tree-dump "return word_\[0-9]*\\(D\\);" "optimized" } } */
Jakub
Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry.
On 05/01/2017 09:13 PM, Jason Merrill wrote:
> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška wrote:
>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote:
>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote:
Hello.
This is patch that was originally installed by Jason and later reverted
due to PR70422.
In the later PR Richi suggested a fix for that and Segher verified that it
helped him
to survive regression tests. That's reason why I'm resending that.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Ready to be installed?
Martin
>>>
>From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001
From: marxin
Date: Thu, 20 Apr 2017 14:56:30 +0200
Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
symbol entry.
gcc/cp/ChangeLog:
2017-04-20 Jason Merrill
Martin Liska
Segher Boessenkool
PR c++/64266
PR c++/70353
PR bootstrap/70422
Core issue 1962
* decl.c (cp_fname_init): Decay the initializer to pointer.
(cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
* pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
DECL_IGNORED_P. Don't call cp_finish_decl.
>>>
>>> If we don't emit those into the debug info, will the debugger be
>>> able to handle __FUNCTION__ etc. properly?
>>
>> No, debugger with the patch can't handled these. Similar to how clang
>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g?
>>
>>> Admittedly, right now we emit it into debug info only if those decls
>>> are actually used, say on:
>>> const char * foo () { return __FUNCTION__; }
>>> const char * bar () { return ""; }
>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger
>>> has to have some handling of it anyway. But while in functions
>>> that don't refer to __FUNCTION__ it is always the debugger that needs
>>> to synthetize those and thus they will be always pointer-equal,
>>> if there are some uses of it and for other uses the debugger would
>>> synthetize it, there is the possibility that the debugger synthetized
>>> string will not be the same object as actually used in the function.
>>
>> You're right, currently one has to use a special function to be able to
>> print it in debugger. I believe we've already discussed that, according
>> to spec, the strings don't have to point to a same string.
>>
>> Suggestions what we should do with the patch?
>
> We need to emit debug information for these variables. From Jim's
> description in 70422 it seems that the problem is that the reference
> to the string from the debug information is breaking
> function_mergeable_rodata_prefix, which relies on
> current_function_decl. It seems to me that its callers should pass
> along their decl parameter so that f_m_r_p can use the decl's
> DECL_CONTEXT rather than rely on current_function_decl being set
> properly>
> Jason
>
Ok, after some time I returned back to it. I followed your advises and
changed the function function_mergeable_rodata_prefix. Apart from a small
rebase was needed.
May I ask Jim to test the patch?
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Martin
>From 8a7a8b421d81c47cf686175f309f79c0b7a8ce76 Mon Sep 17 00:00:00 2001
From: marxin
Date: Thu, 20 Apr 2017 14:56:30 +0200
Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate
symbol entry.
gcc/cp/ChangeLog:
2017-04-20 Jason Merrill
Martin Liska
Segher Boessenkool
PR c++/64266
PR c++/70353
PR bootstrap/70422
Core issue 1962
* decl.c (cp_fname_init): Decay the initializer to pointer.
(cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P,
* pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR,
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and
DECL_IGNORED_P. Don't call cp_finish_decl.
* decl.c (cp_make_fname_decl): Add only non error mark
expression.
gcc/testsuite/ChangeLog:
2017-04-20 Jason Merrill
Segher Boessenkool
PR c++/64266
PR c++/70353
PR bootstrap/70422
Core issue 1962
* g++.dg/cpp0x/constexpr-__func__2.C: Add static assert test.
* g++.dg/ext/fnname5.C: New test.
* g++.old-deja/g++.ext/pretty4.C: Remove.
gcc/ChangeLog:
2017-07-14 Martin Liska
* varasm.c (function_mergeable_rodata_prefix): Use DECL_CONTEXT
for function declarations.
---
gcc/cp/decl.c| 24 +--
gcc/cp/pt.c | 26 +---
gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C | 7 +-
gcc/testsuite/g++.dg/ext/fnname5.C | 33 +
gcc/testsuite/g++.old-deja/g++.ext/pretty4.C | 85
gcc/varasm.c | 7 +-
6 files changed, 76 insertions(+), 106 deletions(-)
create mod
Re: [PATCH 3/3, GCC/ARM] Add support for ARM Cortex-R52 processor
Hi,
On 29/06/17 16:13, Thomas Preudhomme wrote:
Please ignore this patch. I'll respin the patch on a more recent GCC.
Please find an updated patch in attachment.
This patch adds support for the ARM Cortex-R52 processor rencently
announced.
[1] https://developer.arm.com/products/processors/cortex-r/cortex-r52
ChangeLog entry is as follows:
*** gcc/ChangeLog ***
2017-07-14 Thomas Preud'homme
* config/arm/arm-cpus.in (cortex-r52): Add new entry.
(armv8-r): Set ARM Cortex-R52 as default CPU.
* config/arm/arm-tables.opt: Regenerate.
* config/arm/arm-tune.md: Regenerate.
* config/arm/driver-arm.c (arm_cpu_table): Add entry for ARM
Cortex-R52.
* doc/invoke.texi: Mention -mtune=cortex-r52 and availability of fp.dp
extension for -mcpu=cortex-r52.
Tested by building an arm-none-eabi GCC cross-compiler targeting Cortex-R52 and
building an hello world with it. Also checked that the .fpu option created by
GCC for -mcpu=cortex-r52 and -mcpu=cortex-r52+nofp.dp is as expected
(respectively .fpu neon-fp-armv8 and .fpu fpv5-sp-d16
Is this ok for trunk?
Best regards,
Thomas
diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in
index e2ff297aed7514073dbb3bf5ee86964f202e5a14..d009a9e18acb093aefe0f9d8d6de49489fc2325c 100644
--- a/gcc/config/arm/arm-cpus.in
+++ b/gcc/config/arm/arm-cpus.in
@@ -381,7 +381,7 @@ begin arch armv8-m.main
end arch armv8-m.main
begin arch armv8-r
- tune for cortex-r4
+ tune for cortex-r52
tune flags CO_PROC
base 8R
profile R
@@ -1315,6 +1315,16 @@ begin cpu cortex-m33
costs v7m
end cpu cortex-m33
+# V8 R-profile implementations.
+begin cpu cortex-r52
+ cname cortexr52
+ tune flags LDSCHED
+ architecture armv8-r+crc+simd
+ fpu neon-fp-armv8
+ option nofp.dp remove FP_DBL ALL_SIMD
+ costs cortex
+end cpu cortex-r52
+
# FPU entries
# format:
# begin fpu
diff --git a/gcc/config/arm/arm-tables.opt b/gcc/config/arm/arm-tables.opt
index 51678c2566e841894c5c0e9c613c8c0f832e9988..4e508b1555a77628ff6e7cfea39c98b87caa840a 100644
--- a/gcc/config/arm/arm-tables.opt
+++ b/gcc/config/arm/arm-tables.opt
@@ -357,6 +357,9 @@ Enum(processor_type) String(cortex-m23) Value( TARGET_CPU_cortexm23)
EnumValue
Enum(processor_type) String(cortex-m33) Value( TARGET_CPU_cortexm33)
+EnumValue
+Enum(processor_type) String(cortex-r52) Value( TARGET_CPU_cortexr52)
+
Enum
Name(arm_arch) Type(int)
Known ARM architectures (for use with the -march= option):
diff --git a/gcc/config/arm/arm-tune.md b/gcc/config/arm/arm-tune.md
index ba2c7d8ecfdbf6966ebf04b680d587a0e057b161..1b3f7a94cc78fac8abf1042ef60c81a74eaf24eb 100644
--- a/gcc/config/arm/arm-tune.md
+++ b/gcc/config/arm/arm-tune.md
@@ -57,5 +57,6 @@
cortexa73,exynosm1,xgene1,
cortexa57cortexa53,cortexa72cortexa53,cortexa73cortexa35,
cortexa73cortexa53,cortexa55,cortexa75,
- cortexa75cortexa55,cortexm23,cortexm33"
+ cortexa75cortexa55,cortexm23,cortexm33,
+ cortexr52"
(const (symbol_ref "((enum attr_tune) arm_tune)")))
diff --git a/gcc/config/arm/driver-arm.c b/gcc/config/arm/driver-arm.c
index 16171d4e801af46ad549314d1f376e90d5bff57c..5c29b94caaba4ff6f89a191f1d8edcf10431c0b3 100644
--- a/gcc/config/arm/driver-arm.c
+++ b/gcc/config/arm/driver-arm.c
@@ -58,6 +58,7 @@ static struct vendor_cpu arm_cpu_table[] = {
{"0xc15", "armv7-r", "cortex-r5"},
{"0xc17", "armv7-r", "cortex-r7"},
{"0xc18", "armv7-r", "cortex-r8"},
+{"0xd13", "armv8-r+crc", "cortex-r52"},
{"0xc20", "armv6-m", "cortex-m0"},
{"0xc21", "armv6-m", "cortex-m1"},
{"0xc23", "armv7-m", "cortex-m3"},
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e60edcae53ef3c995054b9b0229b5f0fccbb8462..a093b9bcf77b1f4b40992516e853826bb7d528d4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -15538,7 +15538,7 @@ Permissible names are: @samp{arm2}, @samp{arm250},
@samp{cortex-a32}, @samp{cortex-a35}, @samp{cortex-a53}, @samp{cortex-a55},
@samp{cortex-a57}, @samp{cortex-a72}, @samp{cortex-a73}, @samp{cortex-a75},
@samp{cortex-r4}, @samp{cortex-r4f}, @samp{cortex-r5}, @samp{cortex-r7},
-@samp{cortex-r8},
+@samp{cortex-r8}, @samp{cortex-r52},
@samp{cortex-m33},
@samp{cortex-m23},
@samp{cortex-m7},
@@ -15628,7 +15628,7 @@ Disables the floating-point and SIMD instructions on
@item +nofp.dp
Disables the double-precision component of the floating-point instructions
-on @samp{cortex-r5} and @samp{cortex-m7}.
+on @samp{cortex-r5}, @samp{cortex-r52} and @samp{cortex-m7}.
@item +nosimd
Disables the SIMD (but not floating-point) instructions on
[PATCH, GCC/ARM] Rewire -mfpu=fp-armv8 as VFPv5 + D32 + DP
Hi, fp-armv8 is currently defined as a double precision FPv5 with 32 D registers *and* a special FP_ARMv8 bit. However FP for ARMv8 should only bring 32 D registers on top of FPv5-D16 so this FP_ARMv8 bit is spurious. As a consequence, many instruction patterns which are guarded by TARGET_FPU_ARMV8 are unavailable to FPv5-D16 and FPv5-SP-D16. This patch gets rid of TARGET_FPU_ARMV8 and rewire all uses to expressions based on TARGET_VFP5, TARGET_VFPD32 and TARGET_VFP_DOUBLE. It also redefine ISA_FP_ARMv8 to include the D32 capability to distinguish it from FPv5-D16. At last, it sets the +fp.sp for ARMv8-R to enable FPv5-SP-D16 (ie FP for ARMv8 with single precision only and 16 D registers). ChangeLog entry is as follows: 2017-07-07 Thomas Preud'homme * config/arm/arm-isa.h (isa_bit_FP_ARMv8): Delete enumerator. (ISA_FP_ARMv8): Define as ISA_FPv5 and ISA_FP_D32. * config/arm/arm-cpus.in (armv8-r): Define fp.sp as enabling FPv5. (fp-armv8): Define it as FP_ARMv8 only. config/arm/arm.h (TARGET_FPU_ARMV8): Delete. (TARGET_VFP_FP16INST): Define using TARGET_VFP5 rather than TARGET_FPU_ARMV8. config/arm/arm.c (arm_rtx_costs_internal): Replace checks against TARGET_FPU_ARMV8 by checks against TARGET_VFP5. * config/arm/arm-builtins.c (arm_builtin_vectorized_function): Define first ARM_CHECK_BUILTIN_MODE definition using TARGET_VFP5 rather than TARGET_FPU_ARMV8. * config/arm/arm-c.c (arm_cpu_builtins): Likewise for __ARM_FEATURE_NUMERIC_MAXMIN macro definition. * config/arm/arm.md (cmov): Condition on TARGET_VFP5 rather than TARGET_FPU_ARMV8. * config/arm/neon.md (neon_vrint): Likewise. (neon_vcvt): Likewise. (neon_): Likewise. (3): Likewise. * config/arm/vfp.md (lsi2): Likewise. * config/arm/predicates.md (arm_cond_move_operator): Check against TARGET_VFP5 rather than TARGET_FPU_ARMV8 and fix spacing. Testing: * Bootstrapped under ARMv8-A Thumb state and ran testsuite -> no regression * built Spec2000 and Spec2006 with -march=armv8-a+fp16 and compared objdump -> no code generation difference Is this ok for trunk? Best regards, Thomas diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c index 63ee880822c17eda55dd58438d61cbbba333b2c6..7504ed581c63a657a0dff48442633704bd252b2e 100644 --- a/gcc/config/arm/arm-builtins.c +++ b/gcc/config/arm/arm-builtins.c @@ -3098,7 +3098,7 @@ arm_builtin_vectorized_function (unsigned int fn, tree type_out, tree type_in) NULL_TREE is returned if no such builtin is available. */ #undef ARM_CHECK_BUILTIN_MODE #define ARM_CHECK_BUILTIN_MODE(C)\ - (TARGET_FPU_ARMV8 \ + (TARGET_VFP5 \ && flag_unsafe_math_optimizations \ && ARM_CHECK_BUILTIN_MODE_1 (C)) diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c index a3daa3220a2bc4220dffdb7ca08ca9419bdac425..9178937b6d9e0fe5d0948701390c4cf01f4f8c7d 100644 --- a/gcc/config/arm/arm-c.c +++ b/gcc/config/arm/arm-c.c @@ -96,7 +96,7 @@ arm_cpu_builtins (struct cpp_reader* pfile) || TARGET_ARM_ARCH_ISA_THUMB >=2)); def_or_undef_macro (pfile, "__ARM_FEATURE_NUMERIC_MAXMIN", - TARGET_ARM_ARCH >= 8 && TARGET_NEON && TARGET_FPU_ARMV8); + TARGET_ARM_ARCH >= 8 && TARGET_NEON && TARGET_VFP5); def_or_undef_macro (pfile, "__ARM_FEATURE_SIMD32", TARGET_INT_SIMD); diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in index f35128acb7d68c6a0592355b9d3d56ee8f826aca..e2ff297aed7514073dbb3bf5ee86964f202e5a14 100644 --- a/gcc/config/arm/arm-cpus.in +++ b/gcc/config/arm/arm-cpus.in @@ -389,7 +389,7 @@ begin arch armv8-r option crc add bit_crc32 # fp.sp => fp-armv8 (d16); simd => simd + fp-armv8 + d32 + double precision # note: no fp option for fp-armv8 (d16) + double precision at the moment - option fp.sp add FP_ARMv8 + option fp.sp add FPv5 option simd add FP_ARMv8 NEON option crypto add FP_ARMv8 CRYPTO option nocrypto remove ALL_CRYPTO @@ -1390,7 +1390,7 @@ begin fpu fpv5-d16 end fpu fpv5-d16 begin fpu fp-armv8 - isa FP_ARMv8 FP_D32 + isa FP_ARMv8 end fpu fp-armv8 begin fpu neon-fp-armv8 diff --git a/gcc/config/arm/arm-isa.h b/gcc/config/arm/arm-isa.h index 0d66a0400c517668db023fc66ff43e26d43add51..dbd29eaa52f2007498c2aff6263b8b6c3a70e2c2 100644 --- a/gcc/config/arm/arm-isa.h +++ b/gcc/config/arm/arm-isa.h @@ -60,7 +60,6 @@ enum isa_feature isa_bit_VFPv4, /* Vector floating point v4. */ isa_bit_FPv5, /* Floating point v5. */ isa_bit_lpae, /* ARMv7-A LPAE. */ -isa_bit_FP_ARMv8, /* ARMv8 floating-point extension. */ isa_bit_neon, /* Advanced SIMD instructions. */ isa_bit_fp16conv, /* Conversions to/from fp16 (VFPv3 extension). */ isa_bit_fp_dbl, /* Double precision operations supported. */ @@ -143,7 +142,7 @@ enum isa_feature default. isa_bit_fp16 is deliberately missing from this list. */ #define ISA_
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 07/08
On Wed, Jul 12, 2017 at 12:20:32AM +, Wilco Dijkstra wrote: > No - like I mentioned before that's not correct nor acceptable as it would > imply > that ~70% of functions need a probe at entry. I did a quick run across SPEC > and > found the outgoing argument size is > 1024 in just 9 functions out of 147000! > Those 9 were odd special cases due to auto generated code to interface between > C and Fortran. This is extremely unlikely to occur anywhere else. So even > assuming > an unchecked caller, large outgoing arguments are simply not a realistic > threat. > > Therefore even when using a tiny 4K probe size we can safely adjust SP by 3KB > before needing an explicit probe - now only 0.6% of functions need a probe. > If we choose a proper minimum probe distance, say 64KB, explicit probes are > basically non-existent (just 35 functions, or ~0.02% of all functions are > > 64KB). > Clearly inserting probes can be the default as the impact on code quality is > negligible. For non-leaf functions you need at least one probe no matter how small the frame size is (if it is bigger than 0), explicit or implicit, unless you perform IPA analysis on the callgraph and determine when that isn't needed, because you can have deep call stacks that would through functions that don't touch anything skip stack pages. Of course, such probes can be stores of call used registers, it can be any store to the stack. Jakub
Re: [PATCH] v2: c/c++: Add fix-it hints for suggested missing #includes
On 07/03/2017 05:09 PM, David Malcolm wrote: > On Fri, 2017-06-30 at 09:40 -0600, Jeff Law wrote: >> On 05/26/2017 01:54 PM, David Malcolm wrote: >>> Ping: >>> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00321.html >>> >>> On Thu, 2017-05-04 at 12:36 -0400, David Malcolm wrote: As of r247522, fix-it-hints can suggest the insertion of new lines. This patch uses this to implement a new "maybe_add_include_fixit" function in c-common.c and uses it in the two places where the C and C++ frontend can suggest missing #include directives. [1] The idea is that the user can then click on the fix-it in an IDE and have it add the #include for them (or use -fdiagnostics -generate -patch). Examples can be seen in the test cases. The function attempts to put the #include in a reasonable place: immediately after the last #include within the file, or at the top of the file. It is idempotent, so -fdiagnostics-generate -patch does the right thing if several such diagnostics are emitted. Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. OK for trunk? [1] I'm working on a followup which tweaks another diagnostic so that it can suggest that a #include was missing, so I'll use it there as well. gcc/c-family/ChangeLog: * c-common.c (try_to_locate_new_include_insertion_point): New function. (per_file_includes_t): New typedef. (added_includes_t): New typedef. (added_includes): New variable. (maybe_add_include_fixit): New function. * c-common.h (maybe_add_include_fixit): New decl. gcc/c/ChangeLog: * c-decl.c (implicitly_declare): When suggesting a missing #include, provide a fix-it hint. gcc/cp/ChangeLog: * name-lookup.c (get_std_name_hint): Add '<' and '>' around the header names. (maybe_suggest_missing_header): Update for addition of '<' and '>' to above. Provide a fix-it hint. gcc/testsuite/ChangeLog: * g++.dg/lookup/missing-std-include-2.C: New text case. * gcc.dg/missing-header-fixit-1.c: New test case. >> Generally OK. But a few comments on how you find the location for >> where >> to suggest the new #include. >> >> It looks like you're walking the whole table every time?!? Shouldn't >> you at least bound things between start of file and the point where >> an >> error was issued? ie, if you used an undefined type on line XX, a >> #include after line XX makes no sense to resolve the error. >> >> I'm not sure how often this will get called when someone does >> something >> stupid and needs the #include. But ISTM you're looking for two >> bounds. >> >> For the "last" case you start at the statement which generated the >> error >> and walk backwards stopping when you find the last map. >> >> For the "first" case, you start at the beginning and walk forward to >> find the map, then quit. >> >> Are those not appropriate here? > > Here's an updated version of the patch. > > Changed in v2: > > * updated try_to_locate_new_include_insertion_point so that it stops > searching when it reaches the ordinary map containing the location > of the diagnostic, giving an upper bound to the search (see notes > in https://gcc.gnu.org/ml/gcc-patches/2017-06/msg02434.html for more > discussion of this). > * added test coverage for a missing #include within a header (rather than > the main source file). The #include is added to the header in this > case. > * C++: added a couple of fix-it hints to errors that were already > suggested missing includes in the text of the message (for > and ); added test coverage for these. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > OK for trunk? > > gcc/c-family/ChangeLog: > * c-common.c (try_to_locate_new_include_insertion_point): New > function. > (per_file_includes_t): New typedef. > (added_includes_t): New typedef. > (added_includes): New variable. > (maybe_add_include_fixit): New function. > * c-common.h (maybe_add_include_fixit): New decl. > > gcc/c/ChangeLog: > * c-decl.c (implicitly_declare): When suggesting a missing > #include, provide a fix-it hint. > > gcc/cp/ChangeLog: > * name-lookup.c (get_std_name_hint): Add '<' and '>' around > the header names. > (maybe_suggest_missing_header): Update for addition of '<' and '>' > to above. Provide a fix-it hint. > * pt.c: Include "gcc-rich-location.h" > (listify): Attempt to add fix-it hint for missing > #include . > * rtti.c: Include "gcc-rich-location.h". > (typeid_ok_p): Attempt to add fix-it hint for missing > #include . > > gcc/testsuite/ChangeLog: > * g++.dg/cpp0x/missing-initializer_list-include.C: New test case. > * g++.dg/lookup/missing-std-include-2.
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08
On 07/14/2017 01:40 AM, Jakub Jelinek wrote: > On Thu, Jul 13, 2017 at 04:32:02PM -0500, Segher Boessenkool wrote: >> I don't think it should be inside -fstack-check at all. Sure, the >> mechanisms implementing it overlap a bit (more on some targets, less >> on others), but how will a user ask for clash protection _and_ for >> stack checking? > > Are we willing to implement that? What would we do in that case? I'd change every existing target that has a backend stack probing implementatoin to use a moving-sp style. ie, allocate page, probe page, allocate page, probe page. We'd then want to use the -fstack-check routines rather than the new routines. But in the end I don't think using the options together makes all that much sense. We really should look at -fstack-check as Ada specific, even though its implementation is in the target files and middle end. Jeff
Re: [RFC][PATCH] Do refactoring of attribute functions and move them to attribs.[hc].
On 07/14/2017 09:23 AM, Jeff Law wrote: > On 07/13/2017 07:51 AM, Martin Liška wrote: >> Hi. >> >> It's request for comment where I mechanically moved attribute-related >> function to attribs.[hc]. >> >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Thoughts? > Seems reasonable. We don't like to move things around without a good > reason and I assume you've got something in mind? Well, I basically identified that attribute-related functions are spread in between attribs.[ch] and tree.c[ch]. And as the later ones are quite big, the code movement logical to me. I'm going to prepare changelog entry for that. Martin > > jeff >
Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08
On Thu, Jul 13, 2017 at 04:32:02PM -0500, Segher Boessenkool wrote: > I don't think it should be inside -fstack-check at all. Sure, the > mechanisms implementing it overlap a bit (more on some targets, less > on others), but how will a user ask for clash protection _and_ for > stack checking? Are we willing to implement that? What would we do in that case? Probe in each function we emit both the current page that -fstack-check=clash would probe and a page or how many below that how -fstack-check=specific or generic would probe? Even if yes, we can still use -fstack-check=clash,specific for that (similarly to how we support -fsanitize=shift,unreachable). But it is still a stack check kind. Jakub
Re: Add support to trace comparison instructions and switch statements
On 07/10/2017 06:07 AM, 吴潍浠(此彼) wrote: > Hi > > I write some codes to make gcc support comparison-guided fuzzing. > It is very like > http://clang.llvm.org/docs/SanitizerCoverage.html#tracing-data-flow . > With -fsanitize-coverage=trace-cmp the compiler will insert extra > instrumentation around comparison instructions and switch statements. > I think it is useful for fuzzing. :D > > Patch is below, I may supply test cases later. Before anyone can really look at this code you'll need to get a copyright assignment on file with the FSF. See: https://gcc.gnu.org/contribute.html If you've already done this, please let me know and I'll confirm with the FSF copyright clerk. Jeff
Re: [PATCH] Remove Java references in source code.
On 07/11/2017 08:30 AM, Martin Liška wrote: > Hello. > > Patch removes some remaining references in gcc/ subfolder. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Ready to be installed? > > Martin > > gcc/ChangeLog: > > 2017-07-11 Martin Liska > > * cfgexpand.c (expand_gimple_basic_block): Remove dead comment. > * dwarf2out.c (is_java): Remove the function. > (output_pubname): Remove usage of the function. > (lower_bound_default): Remove usage of DW_LANG_Java. > (gen_compile_unit_die): Likewise. > * gcc.c: Remove compiler defaults for .java and .zip files. > * gimple-expr.c (remove_suffix): Change as there's no longer > extension than 4-letter one. > * gimplify.c (mostly_copy_tree_r): Remove Java-special part. > (gimplify_save_expr): Likewise. > * ipa-utils.h (polymorphic_type_binfo_p): Remove the comment > as it's possible even for other languages than Java. > * langhooks.h (struct lang_hooks): Remove Java from a comment. > * lto-opts.c (lto_write_options): Remove reference to Java. > * opts.c (strip_off_ending): Update file extension handling. > * tree-cfg.c (verify_gimple_call): Remove comment with Java. > * tree-eh.c (lower_resx): Likewise. > * tree.c (free_lang_data_in_type): Remove dead code. > (find_decls_types_r): Likewise. > (build_common_builtin_nodes): Remove Java from a comment. > (verify_type): Remove dead code. > * varasm.c (assemble_external): Remove Java from a comment. OK. jeff
Re: [RFC][PATCH] Do refactoring of attribute functions and move them to attribs.[hc].
On 07/13/2017 07:51 AM, Martin Liška wrote: > Hi. > > It's request for comment where I mechanically moved attribute-related > function to attribs.[hc]. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Thoughts? Seems reasonable. We don't like to move things around without a good reason and I assume you've got something in mind? jeff
Re: [PATCH] Add additional quotes to opts.c.
On 07/14/2017 01:00 AM, Martin Liška wrote: > Hi. > > There's a small follow up that I've just tested. > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Ready to be installed? > Martin > > > 0001-Add-additional-quotes-to-opts.c.patch > > > From 712746caf299546f2e643e9e371c077643c46ad6 Mon Sep 17 00:00:00 2001 > From: marxin > Date: Thu, 13 Jul 2017 14:22:50 +0200 > Subject: [PATCH] Add additional quotes to opts.c. > > gcc/ChangeLog: > > 2017-07-13 Martin Liska > > * opts.c (finish_options): Add quotes. > (common_handle_option): Likewise. OK. jeff
Re: [PATCH] Remove Pascal language in source code.
On 07/13/2017 06:06 PM, Pedro Alves wrote: > On 07/13/2017 03:59 PM, Jeff Law wrote: > >> The only concern I'd have here is the bits in dbxout.[ch] might >> effectively be the best documentation of the dbxout format that exists. >> Thus, dropping something like N_SO_PASCAL loses that historical >> documentation. > > FYI, there's a texinfo document in the GDB repo describing the > stabs format, and it documents N_SO_PASCAL: > > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/doc/stabs.texinfo;h=a7ea808a41b290b7dfc4f44801a540a834ee04db;hb=HEAD#l440 > > A pre-generated html version is live here: > > https://www.sourceware.org/gdb/onlinedocs/stabs.html > https://www.sourceware.org/gdb/onlinedocs/stabs.html#Source-Files > > Thanks, > Pedro Alves > Thank you both. After what was said, I installed the patch as r250197. Martin
[PATCH] Add additional quotes to opts.c.
Hi.
There's a small follow up that I've just tested.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Ready to be installed?
Martin
>From 712746caf299546f2e643e9e371c077643c46ad6 Mon Sep 17 00:00:00 2001
From: marxin
Date: Thu, 13 Jul 2017 14:22:50 +0200
Subject: [PATCH] Add additional quotes to opts.c.
gcc/ChangeLog:
2017-07-13 Martin Liska
* opts.c (finish_options): Add quotes.
(common_handle_option): Likewise.
---
gcc/opts.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/gcc/opts.c b/gcc/opts.c
index e2e684fa009..987a7aff3fc 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -823,8 +823,8 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
{
if (opts_set->x_flag_reorder_blocks_and_partition)
inform (loc,
-"-freorder-blocks-and-partition does not work "
-"with exceptions on this architecture");
+ "%<-freorder-blocks-and-partition%> does not work "
+ "with exceptions on this architecture");
opts->x_flag_reorder_blocks_and_partition = 0;
opts->x_flag_reorder_blocks = 1;
}
@@ -839,8 +839,8 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
{
if (opts_set->x_flag_reorder_blocks_and_partition)
inform (loc,
-"-freorder-blocks-and-partition does not support "
-"unwind info on this architecture");
+ "%<-freorder-blocks-and-partition%> does not support "
+ "unwind info on this architecture");
opts->x_flag_reorder_blocks_and_partition = 0;
opts->x_flag_reorder_blocks = 1;
}
@@ -857,8 +857,8 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
{
if (opts_set->x_flag_reorder_blocks_and_partition)
inform (loc,
-"-freorder-blocks-and-partition does not work "
-"on this architecture");
+ "%<-freorder-blocks-and-partition%> does not work "
+ "on this architecture");
opts->x_flag_reorder_blocks_and_partition = 0;
opts->x_flag_reorder_blocks = 1;
}
@@ -895,7 +895,8 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
&& !opts->x_flag_use_linker_plugin)))
{
if (opts_set->x_flag_fat_lto_objects)
-error_at (loc, "-fno-fat-lto-objects are supported only with linker plugin");
+ error_at (loc, "%<-fno-fat-lto-objects%> are supported only with "
+ "linker plugin");
opts->x_flag_fat_lto_objects = 1;
}
}
@@ -2127,7 +2128,7 @@ common_handle_option (struct gcc_options *opts,
#ifndef ACCEL_COMPILER
case OPT_foffload_abi_:
- error_at (loc, "-foffload-abi option can be specified only for "
+ error_at (loc, "%<-foffload-abi%> option can be specified only for "
"offload compiler");
break;
#endif
--
2.13.2
