Re: [patch, libgfortran] PR78387 OpenMP segfault/stack size exceeded writing to internal file
Hi Jerry, ping - I will commit if I hear no objections. OK for trunk and gcc-7. I thought Paul had already OK'd it, which is why I didn't react. Thanks a lot for the patch! Regards Thomas
[PATCH][PING^2] Fix PR81503 (SLSR invalid fold)
Ping.
Thanks!
Bill
> On Aug 14, 2017, at 9:32 AM, Bill Schmidt wrote:
>
> Hi,
>
> I'd like to ping this patch, please.
>
> Thanks!
> Bill
>
>> On Aug 3, 2017, at 2:34 PM, Bill Schmidt wrote:
>>
>> Hi,
>>
>> Here's v2 of the patch with Jakub's suggestions incorporated. Bootstrapped
>> and tested on powerpc64le-linux-gnu with no regressions. Is this ok for
>> trunk?
>>
>> Eventually this should be backported to all active releases as well.
>> Ok for that after a week or so of burn-in? (And after 7.2, I imagine.)
>>
>> Thanks,
>> Bill
>>
>>
>> [gcc]
>>
>> 2017-08-03 Bill Schmidt
>> Jakub Jelinek
>>
>> PR tree-optimization/81503
>> * gimple-ssa-strength-reduction.c (replace_mult_candidate): Ensure
>> folded constant fits in the target type.
>>
>> [gcc/testsuite]
>>
>> 2017-08-03 Bill Schmidt
>> Jakub Jelinek
>>
>> PR tree-optimization/81503
>> * gcc.c-torture/execute/pr81503.c: New file.
>>
>>
>> Index: gcc/gimple-ssa-strength-reduction.c
>> ===
>> --- gcc/gimple-ssa-strength-reduction.c (revision 250791)
>> +++ gcc/gimple-ssa-strength-reduction.c (working copy)
>> @@ -2074,6 +2074,10 @@ replace_mult_candidate (slsr_cand_t c, tree basis_
>> {
>> tree target_type = TREE_TYPE (gimple_assign_lhs (c->cand_stmt));
>> enum tree_code cand_code = gimple_assign_rhs_code (c->cand_stmt);
>> + unsigned int prec = TYPE_PRECISION (target_type);
>> + tree maxval = (POINTER_TYPE_P (target_type)
>> + ? TYPE_MAX_VALUE (sizetype)
>> + : TYPE_MAX_VALUE (target_type));
>>
>> /* It is highly unlikely, but possible, that the resulting
>> bump doesn't fit in a HWI. Abandon the replacement
>> @@ -2082,6 +2086,17 @@ 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 more likely that the bump doesn't fit in the target
>> + type, so check whether constraining it to that type changes
>> + the value. For a signed type, the value mustn't change.
>> + For an unsigned type, the value may only change to a
>> + congruent value (for negative bumps). */
>> + && (TYPE_UNSIGNED (target_type)
>> + ? wi::eq_p (wi::neg_p (bump)
>> + ? bump + wi::to_widest (maxval) + 1
>> + : bump,
>> + wi::zext (bump, prec))
>> + : wi::eq_p (bump, wi::sext (bump, prec)))
>> /* It is not useful to replace casts, copies, negates, or adds of
>> an SSA name and a constant. */
>> && cand_code != SSA_NAME
>> Index: gcc/testsuite/gcc.c-torture/execute/pr81503.c
>> ===
>> --- gcc/testsuite/gcc.c-torture/execute/pr81503.c(nonexistent)
>> +++ gcc/testsuite/gcc.c-torture/execute/pr81503.c(working copy)
>> @@ -0,0 +1,15 @@
>> +unsigned short a = 41461;
>> +unsigned short b = 3419;
>> +int c = 0;
>> +
>> +void foo() {
>> + if (a + b * ~(0 != 5))
>> +c = -~(b * ~(0 != 5)) + 2147483647;
>> +}
>> +
>> +int main() {
>> + foo();
>> + if (c != 2147476810)
>> +return -1;
>> + return 0;
>> +}
>>
>>
>> On 8/3/17 1:02 PM, Bill Schmidt wrote:
On Aug 3, 2017, at 11:39 AM, Jakub Jelinek wrote:
On Thu, Aug 03, 2017 at 11:29:44AM -0500, Bill Schmidt wrote:
>> And, wouldn't it be more readable to use:
>> && (TYPE_UNSIGNED (target_type)
>>? (wi::eq_p (bump, wi::zext (bump, prec))
>> || wi::eq_p (bump + wi::to_widest (maxval) + 1,
>>wi::zext (bump, prec)))
>>: wi::eq_p (bump, wi::sext (bump, prec)))
>> ?
> Probably. As noted, it's all becoming a bit unreadable with too
> much negative logic in a long conditional, so I want to clean that
> up in a follow-up.
>
>> For TYPE_UNSIGNED, do you actually need any restriction?
>> What kind of bump values are wrong for unsigned types and why?
> If the value of the bump is actually larger than the precision of the
> type (not likely except for quite small types), say 2 * (maxval + 1)
> which is congruent to 0, the replacement is wrong.
Ah, ok. Anyway, for unsigned type, perhaps it could be written as:
&& (TYPE_UNSIGNED (target_type)
? wi::eq_p (wi::neg_p (bump) ? bump + wi::to_widest (maxval) + 1
: bump, wi::zext (bump, prec))
: wi::eq_p (bump, wi::sext (bump, prec)))
I mean, if bump >= 0, then the bump + wi::to_widest (maxval) + 1
value has no chance to be equal to zero extended bump, and
for bump < 0 only that one has a chance.
>>> Yeah, that's true. And arguably my case for the really large bump
>>> causing problems is kind of thin, because the program is probably
>>> already broken in that case anyway. But I think I will
[patch, fortran] Warn about suspicious assignment to contiguous pointers
Hello world,
the attached patch warns about the dubious pointer assignments (see
test case for details). I think an unconditional warning is OK
in this case because
- Assigning to a pointer from an obvious non-contiguous target
is not useful at all, that I can see
- Some language laywer will come up with the fact that it is,
in fact, legal if the target is empty or has a single
element only, so a hard error would be a rejects-valid.
However, I can also make this into a warning depending on
-Wall, if this is preferred.
Regresson-tested. OK for trunk?
Regards
Thomas
2017-08-27 Thomas Koenig
PR fortran/49232
* expr.c (gfc_check_pointer_assign): Warn for
suspicious assignments with contiguous pointers.
2017-08-27 Thomas Koenig
PR fortran/49232
* gfortran.dg/contiguous_4.f90: New test.
Index: expr.c
===
--- expr.c (Revision 239977)
+++ expr.c (Arbeitskopie)
@@ -3764,6 +3764,66 @@ gfc_check_pointer_assign (gfc_expr *lvalue, gfc_ex
}
}
+ /* Warn for suspicious assignments like
+
+ pointer, real, dimension(:), contiguous :: p
+ real, dimension(10,10) :: a
+ p => a(:,:,2) or p => a(2:4,:) */
+
+ if (lhs_attr.contiguous)
+{
+ gfc_array_ref *ar;
+ int i;
+ bool do_warn;
+
+ do_warn = false;
+ ar = NULL;
+
+ for (ref = rvalue->ref; ref; ref = ref->next)
+ {
+ if (ref->type == REF_ARRAY)
+ {
+ ar = &ref->u.ar;
+ break;
+ }
+ }
+ if (ar && ar->type == AR_SECTION)
+ {
+
+ for (i = 0; i < ar->dimen; i++)
+ {
+ if (ar->dimen_type[i] == DIMEN_RANGE && ar->stride[i]
+ && (ar->stride[i]->expr_type != EXPR_CONSTANT
+ || (ar->stride[i]->expr_type == EXPR_CONSTANT
+ && mpz_cmp_si (ar->stride[i]->value.integer, 1
+ {
+ do_warn = true;
+ break;
+ }
+ }
+ if (!do_warn && ar->dimen > 1)
+ {
+ for (i = 0; i < ar->dimen - 1; i++)
+ {
+ if ((ar->start[i] && ar->as->lower[i]
+ && gfc_dep_compare_expr (ar->start[i], ar->as->lower[i])
+ != 0)
+ || (ar->end[i] && ar->as->upper[i]
+ && gfc_dep_compare_expr (ar->end[i], ar->as->upper[i])
+ != 0))
+ {
+ do_warn = true;
+ break;
+ }
+ }
+ }
+ }
+ if (do_warn)
+ gfc_warning (0, "Assignment to contiguous pointer from "
+ "possibly non-contiguous target at %L",
+ &rvalue->where);
+}
+
/* Warn if it is the LHS pointer may lives longer than the RHS target. */
if (warn_target_lifetime
&& rvalue->expr_type == EXPR_VARIABLE
! { dg-do compile }
program cont_01_neg
implicit none
real, pointer, contiguous :: r(:)
real, pointer, contiguous :: r2(:,:)
real, target :: x(45)
real, target :: x2(5,9)
integer :: i
x = (/ (real(i),i=1,45) /)
x2 = reshape(x,shape(x2))
r => x(::3) ! { dg-warning "Assignment to contiguous pointer" }
r2 => x2(2:,:) ! { dg-warning "Assignment to contiguous pointer" }
r2 => x2(:,2:3)
end program
Re: [patch, fortran] Warn about suspicious assignment to contiguous pointers
Hi Thomas,
> the attached patch warns about the dubious pointer assignments (see
> test case for details).
thanks for the patch! Sounds like a useful diagnostic.
> I think an unconditional warning is OK
> in this case because
>
> - Assigning to a pointer from an obvious non-contiguous target
> is not useful at all, that I can see
I guess you're talking about a *contiguous* pointer here, and in that
case I would argue that, beyond being "not useful", it's even illegal,
so why not throw a hard error, if we can infer at compile-time that
the target is non-contiguous?
> - Some language laywer will come up with the fact that it is,
> in fact, legal if the target is empty or has a single
> element only, so a hard error would be a rejects-valid.
Should be possible to detect and ignore such cases, shouldn't it?
> However, I can also make this into a warning depending on
> -Wall, if this is preferred.
As explained above, I'd vote for an error (or at least a conditional
warning, so that it can be disabled, like most(?) other gfortran
warnings).
On top, some direct comments to your patch:
+ /* Warn for suspicious assignments like
+
+ pointer, real, dimension(:), contiguous :: p
+ real, dimension(10,10) :: a
+ p => a(:,:,2) or p => a(2:4,:) */
I'm all for good and extensive comments, but instead of writing out
example code here, it might be more concise to just say something
like: "Check for assignments of contiguous pointers to non-contiguous
targets."
+ gfc_array_ref *ar;
+ int i;
+ bool do_warn;
+
+ do_warn = false;
+ ar = NULL;
Maybe add the initialization directly to the declaration here?
gfc_array_ref *ar = NULL;
bool do_warn = false;
The following could be replaced by "gfc_find_array_ref", I guess?
+ for (ref = rvalue->ref; ref; ref = ref->next)
+{
+ if (ref->type == REF_ARRAY)
+{
+ ar = &ref->u.ar;
+ break;
+}
+}
Also I noticed that there is a function called
"gfc_is_simply_contiguous" in expr.c. Could that be useful for your
purpose? (Some of the code in there looks very similar to the logic
you're adding.)
Cheers,
Janus
[PATCH, i386]: Fix PR 81995, error: unrecognizable insn
Matched operands should also have matched predicate...
2017-08-27 Uros Bizjak
PR target/81995
* config/i386/i386.md (*): Change operand 2
predicate to register_operand. Reorder operands.
(*btr): Ditto.
(*_mask): Change operand 3 predicate to register_operand.
(*btr_mask): Ditto.
testsuite/ChangeLog:
2017-08-27 Uros Bizjak
PR target/81995
* gcc.target/i386/pr46091-4.c: Add -mregparm=2 for 32bit targets.
* gcc.target/i386/pr46091-4a.c: Ditto.
Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Committed to mainline SVN.
Uros.
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 251368)
+++ config/i386/i386.md (working copy)
@@ -11011,11 +11011,11 @@
[(set (match_operand:SWI48 0 "register_operand" "=r")
(any_or:SWI48
(ashift:SWI48 (const_int 1)
- (match_operand:QI 1 "register_operand" "r"))
- (match_operand:SWI48 2 "nonimmediate_operand" "0")))
+ (match_operand:QI 2 "register_operand" "r"))
+ (match_operand:SWI48 1 "register_operand" "0")))
(clobber (reg:CC FLAGS_REG))]
"TARGET_USE_BT"
- "{}\t{%1, %0|%0, %1}"
+ "{}\t{%2, %0|%0, %2}"
[(set_attr "type" "alu1")
(set_attr "prefix_0f" "1")
(set_attr "znver1_decode" "double")
@@ -11031,7 +11031,7 @@
(and:SI
(match_operand:SI 1 "register_operand")
(match_operand:SI 2 "const_int_operand")) 0))
- (match_operand:SWI48 3 "nonimmediate_operand")))
+ (match_operand:SWI48 3 "register_operand")))
(clobber (reg:CC FLAGS_REG))]
"(INTVAL (operands[2]) & (GET_MODE_BITSIZE (mode)-1))
== GET_MODE_BITSIZE (mode)-1
@@ -11051,11 +11051,11 @@
[(set (match_operand:SWI48 0 "register_operand" "=r")
(and:SWI48
(rotate:SWI48 (const_int -2)
- (match_operand:QI 1 "register_operand" "r"))
- (match_operand:SWI48 2 "nonimmediate_operand" "0")))
+ (match_operand:QI 2 "register_operand" "r"))
+ (match_operand:SWI48 1 "register_operand" "0")))
(clobber (reg:CC FLAGS_REG))]
"TARGET_USE_BT"
- "btr{}\t{%1, %0|%0, %1}"
+ "btr{}\t{%2, %0|%0, %2}"
[(set_attr "type" "alu1")
(set_attr "prefix_0f" "1")
(set_attr "znver1_decode" "double")
@@ -11071,7 +11071,7 @@
(and:SI
(match_operand:SI 1 "register_operand")
(match_operand:SI 2 "const_int_operand")) 0))
- (match_operand:SWI48 3 "nonimmediate_operand")))
+ (match_operand:SWI48 3 "register_operand")))
(clobber (reg:CC FLAGS_REG))]
"(INTVAL (operands[2]) & (GET_MODE_BITSIZE (mode)-1))
== GET_MODE_BITSIZE (mode)-1
Index: testsuite/gcc.target/i386/pr46091-4.c
===
--- testsuite/gcc.target/i386/pr46091-4.c (revision 251368)
+++ testsuite/gcc.target/i386/pr46091-4.c (working copy)
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2" } */
+/* { dg-additional-options "-mregparm=2" { target ia32 } } */
int test_1 (int x, int n)
{
Index: testsuite/gcc.target/i386/pr46091-4a.c
===
--- testsuite/gcc.target/i386/pr46091-4a.c (revision 251368)
+++ testsuite/gcc.target/i386/pr46091-4a.c (working copy)
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-O2" } */
+/* { dg-additional-options "-mregparm=2" { target ia32 } } */
int test_1 (int x, int n)
{
[RFC, vectorizer] Allow single element vector types for vector reduction operations
Hi,
I have an out-of-tree GCC port and it is struggling supporting
auto-vectorization on some dot product instructions. For example, I have an
instruction that takes three operands which are all 32-bit general
registers. The second and third operands will be treated as V2HI then do dot
product, and then generate an SI result which is then added to the first
operand which is SI as well.
I do see there is dot product recognizer in tree-vect-patters.c, however, I
found the following testcase still can't be auto-vectorized on my port which
has implemented all necessary dot product standard patterns. This testcase
can't be auto-vectorized on other targets that have similar V2HI dot product
instructions as well, for example ARC.
=== test.c ===
#define K 4
#define M 4
#define N 256
int in[N*K][M];
int out[K];
int coeff[N][M];
void
foo (void)
{
int i, j, k;
int sum;
for (k = 0; k < K; k++)
{
sum = 0;
for (j = 0; j < M; j++)
for (i = 0; i < N; i++)
sum += in[i+k][j] * coeff[i][j];
out[k] = sum;
}
}
===
The reason that auto-vectorizer doesn't work seems to be that GCC doesn't
support single-element vector types in get_vectype_for_scalar_type_and_size.
tree-vect-stmts.c: get_vectype_for_scalar_type_and_size
...
if (nunits <= 1)
return NULL_TREE;
So, I am thinking this actually should be relaxed to support more cases. At
least on vector reduction operations which normally will have scalar result
with wider types than the element type of input operands.
I have tried to make the auto-vectorizer work for my V2HI dot product case,
with the patch attached. Is this the correct approach?
Cheers,
Jon
gcc/
2017-08-27 Jon Beniston
* tree-vectorizer.h (get_vectype_for_scalar_type): New optional
parameter declaration.
* tree-vect-stmts.c (get_vectype_for_scalar_type_and_size): Add new
optional parameter "reduct_p". Support single element vector types
if it is true.
(get_vectype_for_scalar_type): Add new parameter "reduct_p".
* tree-vect-patterns.c (vect_pattern_recog_1): Pass new parameter
"reduct_p".
* tree-vect-loop.c (vect_determine_vectorization_factor): Likewise.
(vect_model_reduction_cost): Likewise.
(get_initial_def_for_induction): Likewise.
(vect_create_epilog_for_reduction): Likewise.
fix-vec-reduct.patch
Description: Binary data
[Patch, Fortran] PR 81770: [5/6/7 Regression] Bogus warning: Pointer in pointer assignment might outlive the pointer target
Hi all,
the attached patch fixes a bogus warning. The purpose of the warning
is to detect cases where a pointer lives longer than its target. If
the target itself is (1) a pointer or (2) a component of a DT pointer,
we do not know about the lifetime of the target at compile time and no
warning should be thrown. The existing check only handles case (1) and
my patch adds the handling of case (2).
Regtestes cleanly on x86_64-linux-gnu. Ok for trunk and the release branches?
Cheers,
Janus
2017-08-27 Janus Weil
PR fortran/81770
* expr.c (gfc_check_pointer_assign): Improve the check whether pointer
may outlive pointer target.
2017-08-27 Janus Weil
PR fortran/81770
* gfortran.dg/warn_target_lifetime_4.f90: New testcase.
Index: gcc/fortran/expr.c
===
--- gcc/fortran/expr.c (revision 251368)
+++ gcc/fortran/expr.c (working copy)
@@ -3806,7 +3806,8 @@ gfc_check_pointer_assign (gfc_expr *lvalue, gfc_ex
if (warn_target_lifetime
&& rvalue->expr_type == EXPR_VARIABLE
&& !rvalue->symtree->n.sym->attr.save
- && !attr.pointer && !rvalue->symtree->n.sym->attr.host_assoc
+ && !rvalue->symtree->n.sym->attr.pointer && !attr.pointer
+ && !rvalue->symtree->n.sym->attr.host_assoc
&& !rvalue->symtree->n.sym->attr.in_common
&& !rvalue->symtree->n.sym->attr.use_assoc
&& !rvalue->symtree->n.sym->attr.dummy)
! { dg-do compile }
! { dg-options "-Wtarget-lifetime" }
!
! PR fortran/81770: [5/6/7 Regression] Bogus warning: Pointer in pointer assignment might outlive the pointer target
!
! Contributed by Janus Weil
module m
type t
integer, allocatable :: l
end type
contains
subroutine sub(c_in, list)
type(t), target, intent(in) :: c_in
integer, pointer, intent(out) :: list
type(t), pointer :: container
container => c_in
list => container%l
end subroutine
end
