[PATCH] doc clarification: DONE and FAIL in define_split and define_peephole2
Currently DONE and FAIL are documented only for define_expand, but they also
work in essentially the same way for define_split and define_peephole2.
If FAIL is used in a define_insn_and_split, the output pattern cannot be the
usual "#" dummy value.
This patch updates the doc to describe those cases. Ok for trunk?
paul
ChangeLog:
2018-07-05 Paul Koning
* doc/md.texi (define_split): Document DONE and FAIL. Describe
interaction with usual "#" output template in
define_insn_and_split.
(define_peephole2): Document DONE and FAIL.
Index: doc/md.texi
===
--- doc/md.texi (revision 262455)
+++ doc/md.texi (working copy)
@@ -8060,6 +8060,30 @@ those in @code{define_expand}, however, these stat
generate any new pseudo-registers. Once reload has completed, they also
must not allocate any space in the stack frame.
+There are two special macros defined for use in the preparation statements:
+@code{DONE} and @code{FAIL}. Use them with a following semicolon,
+as a statement.
+
+@table @code
+
+@findex DONE
+@item DONE
+Use the @code{DONE} macro to end RTL generation for the splitter. The
+only RTL insns generated as replacement for the matched input insn will
+be those already emitted by explicit calls to @code{emit_insn} within
+the preparation statements; the replacement pattern is not used.
+
+@findex FAIL
+@item FAIL
+Make the @code{define_split} fail on this occasion. When a @code{define_split}
+fails, it means that the splitter was not truly available for the inputs
+it was given, and this split is not done.
+@end table
+
+If the preparation falls through (invokes neither @code{DONE} nor
+@code{FAIL}), then the @code{define_split} uses the replacement
+template.
+
Patterns are matched against @var{insn-pattern} in two different
circumstances. If an insn needs to be split for delay slot scheduling
or insn scheduling, the insn is already known to be valid, which means
@@ -8232,6 +8256,15 @@ functionality as two separate @code{define_insn} a
patterns. It exists for compactness, and as a maintenance tool to prevent
having to ensure the two patterns' templates match.
+In @code{define_insn_and_split}, the output template is usually simply
+@samp{#} since the assembly output is done by @code{define_insn}
+statements matching the generated insns, not by this
+@code{define_insn_and_split} statement. But if @code{FAIL} is used in
+the preparation statements for certain input insns, those will not be
+split and during assembly output will again match this
+@code{define_insn_and_split}. In that case, the appropriate assembly
+output statements are needed in the output template.
+
@end ifset
@ifset INTERNALS
@node Including Patterns
@@ -8615,6 +8648,31 @@ so here's a silly made-up example:
"")
@end smallexample
+There are two special macros defined for use in the preparation statements:
+@code{DONE} and @code{FAIL}. Use them with a following semicolon,
+as a statement.
+
+@table @code
+
+@findex DONE
+@item DONE
+Use the @code{DONE} macro to end RTL generation for the peephole. The
+only RTL insns generated as replacement for the matched input insn will
+be those already emitted by explicit calls to @code{emit_insn} within
+the preparation statements; the replacement pattern is not used.
+
+@findex FAIL
+@item FAIL
+Make the @code{define_peephole2} fail on this occasion. When a
@code{define_peephole2}
+fails, it means that the replacement was not truly available for the
+particular inputs it was given, and the input insns are left unchanged.
+@end table
+
+If the preparation falls through (invokes neither @code{DONE} nor
+@code{FAIL}), then the @code{define_peephole2} uses the replacement
+template.
+
+
@noindent
If we had not added the @code{(match_dup 4)} in the middle of the input
sequence, it might have been the case that the register we chose at the
Re: [PATCH] doc clarification: DONE and FAIL in define_split and define_peephole2
Thanks Richard. Some comments, and an updated proposed patch.
paul
> On Jul 6, 2018, at 9:04 AM, Richard Sandiford
> wrote:
>
> ...
>> @@ -8232,6 +8256,15 @@ functionality as two separate @code{define_insn} a
>> patterns. It exists for compactness, and as a maintenance tool to prevent
>> having to ensure the two patterns' templates match.
>>
>> +In @code{define_insn_and_split}, the output template is usually simply
>> +@samp{#} since the assembly output is done by @code{define_insn}
>> +statements matching the generated insns, not by this
>> +@code{define_insn_and_split} statement. But if @code{FAIL} is used in
>> +the preparation statements for certain input insns, those will not be
>> +split and during assembly output will again match this
>> +@code{define_insn_and_split}. In that case, the appropriate assembly
>> +output statements are needed in the output template.
>> +
>
> I agree "#" on its own is relatively common, but it's also not that
> unusual to have a define_insn_and_split in which the define_insn part
> handles simple alternatives directly and leaves more complex ones to
> be split. Maybe that's more common on RISC-like targets.
>
> Also, the define_split matches the template independently of the
> define_insn, so it can sometimes split insns that match an earlier
> define_insn rather than the one in the define_insn_and_split.
> (That might be bad practice.) So using "#" and FAIL together is valid
> if the FAIL only happens for cases that match earlier define_insns.
>
> Another case is when the define_split condition doesn't start with
> "&&" and is less strict than the define_insn condition. This can
> be useful if the define_split is supposed to match patterns created
> by combine.
>
> So maybe we should instead expand the FAIL documentation to say that
> a define_split must not FAIL when splitting an instruction whose
> output template is "#".
I played with this a bit and couldn't come up with a good wording. The case is
already covered; I was trying to make it clearer but your comments indicate the
picture is bigger than I thought.
>> @@ -8615,6 +8648,31 @@ so here's a silly made-up example:
>> "")
>> @end smallexample
>>
>> +There are two special macros defined for use in the preparation statements:
>> +@code{DONE} and @code{FAIL}. Use them with a following semicolon,
>> +as a statement.
>> +
>> +@table @code
>> +
>> +@findex DONE
>> +@item DONE
>> +Use the @code{DONE} macro to end RTL generation for the peephole. The
>> +only RTL insns generated as replacement for the matched input insn will
>> +be those already emitted by explicit calls to @code{emit_insn} within
>> +the preparation statements; the replacement pattern is not used.
>> +
>> +@findex FAIL
>> +@item FAIL
>> +Make the @code{define_peephole2} fail on this occasion. When a
>> @code{define_peephole2}
>> +fails, it means that the replacement was not truly available for the
>> +particular inputs it was given, and the input insns are left unchanged.
>
> If it FAILs, GCC will try to apply later define_peehole2s instead.
> (This is in contrast to define_split, so it's a bit inconsistent.
> Would be easy to make define_split behave the same way if there was a
> motivating case.)
Interesting. I added words to the FAIL description of both define_split and
define_peephole2 to state the behavior and highlight the difference.
ChangeLog:
2018-07-05 Paul Koning
* doc/md.texi (define_split): Document DONE and FAIL. Describe
interaction with usual "#" output template in
define_insn_and_split.
(define_peephole2): Document DONE and FAIL.
Index: doc/md.texi
===
--- doc/md.texi (revision 262455)
+++ doc/md.texi (working copy)
@@ -8060,6 +8060,30 @@ those in @code{define_expand}, however, these stat
generate any new pseudo-registers. Once reload has completed, they also
must not allocate any space in the stack frame.
+There are two special macros defined for use in the preparation statements:
+@code{DONE} and @code{FAIL}. Use them with a following semicolon,
+as a statement.
+
+@table @code
+
+@findex DONE
+@item DONE
+Use the @code{DONE} macro to end RTL generation for the splitter. The
+only RTL insns generated as replacement for the matched input insn will
+be those already emitted by explicit calls to @code{emit_insn} within
+the preparation statements; the replacement pattern is not used.
+
+@findex FAIL
+@item FAIL
+Make the @code{define_split} fail on this occasion. When a @code{define_sp
Re: [PATCH] doc clarification: DONE and FAIL in define_split and define_peephole2
> On Jul 6, 2018, at 12:20 PM, Richard Sandiford
> wrote:
>
> Double empty line.
>
> OK otherwise, thanks. (Think this counts as a gen* patch.)
>
> Richard
Thanks. Committed as shown below.
paul
ChangeLog:
2018-07-06 Paul Koning
* doc/md.texi (define_split): Document DONE and FAIL.
(define_peephole2): Ditto.
Index: doc/md.texi
===
--- doc/md.texi (revision 262478)
+++ doc/md.texi (working copy)
@@ -8060,6 +8060,30 @@ those in @code{define_expand}, however, these stat
generate any new pseudo-registers. Once reload has completed, they also
must not allocate any space in the stack frame.
+There are two special macros defined for use in the preparation statements:
+@code{DONE} and @code{FAIL}. Use them with a following semicolon,
+as a statement.
+
+@table @code
+
+@findex DONE
+@item DONE
+Use the @code{DONE} macro to end RTL generation for the splitter. The
+only RTL insns generated as replacement for the matched input insn will
+be those already emitted by explicit calls to @code{emit_insn} within
+the preparation statements; the replacement pattern is not used.
+
+@findex FAIL
+@item FAIL
+Make the @code{define_split} fail on this occasion. When a @code{define_split}
+fails, it means that the splitter was not truly available for the inputs
+it was given, and the input insn will not be split.
+@end table
+
+If the preparation falls through (invokes neither @code{DONE} nor
+@code{FAIL}), then the @code{define_split} uses the replacement
+template.
+
Patterns are matched against @var{insn-pattern} in two different
circumstances. If an insn needs to be split for delay slot scheduling
or insn scheduling, the insn is already known to be valid, which means
@@ -8615,6 +8639,33 @@ so here's a silly made-up example:
"")
@end smallexample
+There are two special macros defined for use in the preparation statements:
+@code{DONE} and @code{FAIL}. Use them with a following semicolon,
+as a statement.
+
+@table @code
+
+@findex DONE
+@item DONE
+Use the @code{DONE} macro to end RTL generation for the peephole. The
+only RTL insns generated as replacement for the matched input insn will
+be those already emitted by explicit calls to @code{emit_insn} within
+the preparation statements; the replacement pattern is not used.
+
+@findex FAIL
+@item FAIL
+Make the @code{define_peephole2} fail on this occasion. When a
@code{define_peephole2}
+fails, it means that the replacement was not truly available for the
+particular inputs it was given. In that case, GCC may still apply a
+later @code{define_peephole2} that also matches the given insn pattern.
+(Note that this is different from @code{define_split}, where @code{FAIL}
+prevents the input insn from being split at all.)
+@end table
+
+If the preparation falls through (invokes neither @code{DONE} nor
+@code{FAIL}), then the @code{define_peephole2} uses the replacement
+template.
+
@noindent
If we had not added the @code{(match_dup 4)} in the middle of the input
sequence, it might have been the case that the register we chose at the
[PATCH, doc] Small clarification on define_subst
In doing CCmode work I was confused how define_subst handles cases where the
same argument appears more than once. The attached clarifies this.
Ok for trunk?
paul
ChangeLog:
2018-07-08 Paul Koning
* doc/md.texi (define_subst): Document how multiple occurrences of
the same argument in the replacement pattern are handled.
Index: doc/md.texi
===
--- doc/md.texi (revision 262505)
+++ doc/md.texi (working copy)
@@ -10263,7 +10263,11 @@ the expression from the original pattern, which ma
@code{match_operand N} from the input pattern. As a consequence,
@code{match_dup} cannot be used to point to @code{match_operand}s from
the output pattern, it should always refer to a @code{match_operand}
-from the input pattern.
+from the input pattern. If a @code{match_dup N} occurs more than once
+in the output template, its first occurrence is replaced with the
+expression from the original pattern, and the subsequent expressions
+are replaced with @code{match_dup N}, i.e., a reference to the first
+expression.
In the output template one can refer to the expressions from the
original pattern and create new ones. For instance, some operands could
[PATCH, committed] Improve code generation for pdp11 target
This patch improves the generated code for the pdp11 target. Committed. paul ChangeLog: 2018-07-09 Paul Koning * config/pdp11/pdp11.c (pdp11_addr_cost): New function. (pdp11_insn_cost): New function. (pdp11_md_asm_adjust): New function. (TARGET_INVALID_WITHIN_DOLOOP): Define. (pdp11_rtx_costs): Update to match machine better. (output_addr_const_pdp11): Correct format mismatch warnings. * config/pdp11/pdp11.h (SLOW_BYTE_ACCESS): Correct definition. * config/pdp11/pdp11.md: General change to add base_cost and/or length attributes for use by new pdp11_insn_cost function. (MIN_BRANCH): Correct definition. (MIN_SOB): Ditto. (doloop_end): Use standard pattern name for looping pattern. (doloop_end_nocc): New. (movsf): Add another constraint alternative. (zero_extendqihi2): Add constraint alternatives for not in place extend. (zero_extendhisi2): Remove. (shift patterns): Add CC handling variants. (bswaphi2): New. (bswapsi2): New. (rothi3): New. (define_peephole2): New peephole to recognize mov that sets CC for subsequent test. Index: config/pdp11/pdp11.c === --- config/pdp11/pdp11.c(revision 262518) +++ config/pdp11/pdp11.c(working copy) @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see #include "memmodel.h" #include "tm_p.h" #include "insn-config.h" +#include "insn-attr.h" #include "regs.h" #include "emit-rtl.h" #include "recog.h" @@ -150,6 +151,11 @@ decode_pdp11_d (const struct real_format *fmt ATTR static const char *singlemove_string (rtx *); static bool pdp11_assemble_integer (rtx, unsigned int, int); static bool pdp11_rtx_costs (rtx, machine_mode, int, int, int *, bool); +static int pdp11_addr_cost (rtx, machine_mode, addr_space_t, bool); +static int pdp11_insn_cost (rtx_insn *insn, bool speed); +static rtx_insn *pdp11_md_asm_adjust (vec &, vec &, + vec &, + vec &, HARD_REG_SET &); static bool pdp11_return_in_memory (const_tree, const_tree); static rtx pdp11_function_value (const_tree, const_tree, bool); static rtx pdp11_libcall_value (machine_mode, const_rtx); @@ -174,6 +180,8 @@ static bool pdp11_scalar_mode_supported_p (scalar_ #undef TARGET_ASM_INTEGER #define TARGET_ASM_INTEGER pdp11_assemble_integer +/* These two apply to Unix and GNU assembler; for DEC, they are + overridden during option processing. */ #undef TARGET_ASM_OPEN_PAREN #define TARGET_ASM_OPEN_PAREN "[" #undef TARGET_ASM_CLOSE_PAREN @@ -182,6 +190,15 @@ static bool pdp11_scalar_mode_supported_p (scalar_ #undef TARGET_RTX_COSTS #define TARGET_RTX_COSTS pdp11_rtx_costs +#undef TARGET_ADDRESS_COST +#define TARGET_ADDRESS_COST pdp11_addr_cost + +#undef TARGET_INSN_COST +#define TARGET_INSN_COST pdp11_insn_cost + +#undef TARGET_MD_ASM_ADJUST +#define TARGET_MD_ASM_ADJUST pdp11_md_asm_adjust + #undef TARGET_FUNCTION_ARG #define TARGET_FUNCTION_ARG pdp11_function_arg #undef TARGET_FUNCTION_ARG_ADVANCE @@ -271,6 +288,9 @@ static bool pdp11_scalar_mode_supported_p (scalar_ #undef TARGET_CAN_CHANGE_MODE_CLASS #define TARGET_CAN_CHANGE_MODE_CLASS pdp11_can_change_mode_class + +#undef TARGET_INVALID_WITHIN_DOLOOP +#define TARGET_INVALID_WITHIN_DOLOOP hook_constcharptr_const_rtx_insn_null /* A helper function to determine if REGNO should be saved in the current function's stack frame. */ @@ -968,12 +988,8 @@ pdp11_assemble_integer (rtx x, unsigned int size, } -/* Register to register moves are cheap if both are general registers. - The same is true for FPU, but there we return cost of 3 rather than - 2 to make reload look at the constraints. The raeson is that - load/store double require extra care since load touches condition - codes and store doesn't, which is (partly anyway) described by - constraints. */ +/* Register to register moves are cheap if both are general + registers. */ static int pdp11_register_move_cost (machine_mode mode ATTRIBUTE_UNUSED, reg_class_t c1, reg_class_t c2) @@ -983,151 +999,270 @@ pdp11_register_move_cost (machine_mode mode ATTRIB return 2; else if ((c1 >= LOAD_FPU_REGS && c1 <= FPU_REGS && c2 == LOAD_FPU_REGS) || (c2 >= LOAD_FPU_REGS && c2 <= FPU_REGS && c1 == LOAD_FPU_REGS)) -return 3; +return 2; else return 22; } - +/* This tries to approximate what pdp11_insn_cost would do, but + without visibility into the actual instruction being generated it's + inevitably a rough approximation. */ static bool -pdp11_rtx_costs (rtx x, machine_mode mode, int outer_code ATT
[PATCH] doc: add missing "mode" type attribute
"mode" is documented as a variable attribute but not as a type attribute. This
fixes that omission. I simply copied the other text, it seemed suitable as it
stands.
The attributes are normally listed in alphabetical order but "mode" was out of
order in the variable attributes.
Ok for trunk?
paul
ChangeLog:
2018-07-10 Paul Koning
* doc/extend.texi (Common Variable Attributes): Move "mode" into
alphabetical order.
(Common Type Attributes): Add "mode" attribute.
Index: doc/extend.texi
===
--- doc/extend.texi (revision 262540)
+++ doc/extend.texi (working copy)
@@ -6123,6 +6123,19 @@ types (@pxref{Common Function Attributes},
The message attached to the attribute is affected by the setting of
the @option{-fmessage-length} option.
+@item mode (@var{mode})
+@cindex @code{mode} variable attribute
+This attribute specifies the data type for the declaration---whichever
+type corresponds to the mode @var{mode}. This in effect lets you
+request an integer or floating-point type according to its width.
+
+@xref{Machine Modes,,, gccint, GNU Compiler Collection (GCC) Internals},
+for a list of the possible keywords for @var{mode}.
+You may also specify a mode of @code{byte} or @code{__byte__} to
+indicate the mode corresponding to a one-byte integer, @code{word} or
+@code{__word__} for the mode of a one-word integer, and @code{pointer}
+or @code{__pointer__} for the mode used to represent pointers.
+
@item nonstring
@cindex @code{nonstring} variable attribute
The @code{nonstring} variable attribute specifies that an object or member
@@ -6158,19 +6171,6 @@ int f (struct Data *pd, const char *s)
@}
@end smallexample
-@item mode (@var{mode})
-@cindex @code{mode} variable attribute
-This attribute specifies the data type for the declaration---whichever
-type corresponds to the mode @var{mode}. This in effect lets you
-request an integer or floating-point type according to its width.
-
-@xref{Machine Modes,,, gccint, GNU Compiler Collection (GCC) Internals},
-for a list of the possible keywords for @var{mode}.
-You may also specify a mode of @code{byte} or @code{__byte__} to
-indicate the mode corresponding to a one-byte integer, @code{word} or
-@code{__word__} for the mode of a one-word integer, and @code{pointer}
-or @code{__pointer__} for the mode used to represent pointers.
-
@item packed
@cindex @code{packed} variable attribute
The @code{packed} attribute specifies that a variable or structure field
@@ -7112,6 +7112,19 @@ declaration, the above program would abort when co
@option{-fstrict-aliasing}, which is on by default at @option{-O2} or
above.
+@item mode (@var{mode})
+@cindex @code{mode} type attribute
+This attribute specifies the data type for the declaration---whichever
+type corresponds to the mode @var{mode}. This in effect lets you
+request an integer or floating-point type according to its width.
+
+@xref{Machine Modes,,, gccint, GNU Compiler Collection (GCC) Internals},
+for a list of the possible keywords for @var{mode}.
+You may also specify a mode of @code{byte} or @code{__byte__} to
+indicate the mode corresponding to a one-byte integer, @code{word} or
+@code{__word__} for the mode of a one-word integer, and @code{pointer}
+or @code{__pointer__} for the mode used to represent pointers.
+
@item packed
@cindex @code{packed} type attribute
This attribute, attached to @code{struct} or @code{union} type
Index: doc/md.texi
===
--- doc/md.texi (revision 262540)
+++ doc/md.texi (working copy)
@@ -10263,7 +10263,11 @@ the expression from the original pattern, which ma
@code{match_operand N} from the input pattern. As a consequence,
@code{match_dup} cannot be used to point to @code{match_operand}s from
the output pattern, it should always refer to a @code{match_operand}
-from the input pattern.
+from the input pattern. If a @code{match_dup N} occurs more than once
+in the output template, its first occurrence is replaced with the
+expression from the original pattern, and the subsequent expressions
+are replaced with @code{match_dup N}, i.e., a reference to the first
+expression.
In the output template one can refer to the expressions from the
original pattern and create new ones. For instance, some operands could
[PATCH] doc: update looping constructs
This patch removes the obsolete documentation for
decrement_and_branch_until_zero. It also adds detail to the description for
doloop_end. In particular, it describes the required form of the conditional
branch part of the pattern.
Ok for trunk?
paul
ChangeLog:
2018-07-11 Paul Koning
* doc/rtl.texi (REG_NONNEG): Remove decrement and branch until
zero reference, add doloop_end instead.
* doc/md.texi (decrement_and_branch_until_zero): Remove.
(Looping patterns): Remove decrement_and_branch_until_zero. Add
detail for doloop_end.
Index: doc/md.texi
===
--- doc/md.texi (revision 262562)
+++ doc/md.texi (working copy)
@@ -6681,17 +6681,6 @@ second operand, but you should incorporate it in t
that the jump optimizer will not delete the table as unreachable code.
-@cindex @code{decrement_and_branch_until_zero} instruction pattern
-@item @samp{decrement_and_branch_until_zero}
-Conditional branch instruction that decrements a register and
-jumps if the register is nonzero. Operand 0 is the register to
-decrement and test; operand 1 is the label to jump to if the
-register is nonzero. @xref{Looping Patterns}.
-
-This optional instruction pattern is only used by the combiner,
-typically for loops reversed by the loop optimizer when strength
-reduction is enabled.
-
@cindex @code{doloop_end} instruction pattern
@item @samp{doloop_end}
Conditional branch instruction that decrements a register and
@@ -7515,67 +7504,12 @@ iterations. This avoids the need for fetching and
@samp{dbra}-like instruction and avoids pipeline stalls associated with
the jump.
-GCC has three special named patterns to support low overhead looping.
-They are @samp{decrement_and_branch_until_zero}, @samp{doloop_begin},
-and @samp{doloop_end}. The first pattern,
-@samp{decrement_and_branch_until_zero}, is not emitted during RTL
-generation but may be emitted during the instruction combination phase.
-This requires the assistance of the loop optimizer, using information
-collected during strength reduction, to reverse a loop to count down to
-zero. Some targets also require the loop optimizer to add a
-@code{REG_NONNEG} note to indicate that the iteration count is always
-positive. This is needed if the target performs a signed loop
-termination test. For example, the 68000 uses a pattern similar to the
-following for its @code{dbra} instruction:
+GCC has two special named patterns to support low overhead looping.
+They are @samp{doloop_begin} and @samp{doloop_end}. These are emitted
+by the loop optimizer for certain well-behaved loops with a finite
+number of loop iterations using information collected during strength
+reduction.
-@smallexample
-@group
-(define_insn "decrement_and_branch_until_zero"
- [(set (pc)
-(if_then_else
- (ge (plus:SI (match_operand:SI 0 "general_operand" "+d*am")
- (const_int -1))
- (const_int 0))
- (label_ref (match_operand 1 "" ""))
- (pc)))
- (set (match_dup 0)
-(plus:SI (match_dup 0)
- (const_int -1)))]
- "find_reg_note (insn, REG_NONNEG, 0)"
- "@dots{}")
-@end group
-@end smallexample
-
-Note that since the insn is both a jump insn and has an output, it must
-deal with its own reloads, hence the `m' constraints. Also note that
-since this insn is generated by the instruction combination phase
-combining two sequential insns together into an implicit parallel insn,
-the iteration counter needs to be biased by the same amount as the
-decrement operation, in this case @minus{}1. Note that the following similar
-pattern will not be matched by the combiner.
-
-@smallexample
-@group
-(define_insn "decrement_and_branch_until_zero"
- [(set (pc)
-(if_then_else
- (ge (match_operand:SI 0 "general_operand" "+d*am")
- (const_int 1))
- (label_ref (match_operand 1 "" ""))
- (pc)))
- (set (match_dup 0)
-(plus:SI (match_dup 0)
- (const_int -1)))]
- "find_reg_note (insn, REG_NONNEG, 0)"
- "@dots{}")
-@end group
-@end smallexample
-
-The other two special looping patterns, @samp{doloop_begin} and
-@samp{doloop_end}, are emitted by the loop optimizer for certain
-well-behaved loops with a finite number of loop iterations using
-information collected during strength reduction.
-
The @samp{doloop_end} pattern describes the actual looping instruction
(or the implicit looping operation) and the @samp{doloop_begin} pattern
is an optional companion pattern that can be used for initialization
@@ -7594,15 +7528,65 @@ desired special iteration counter register was not
machine dependent reorg pass could emit a traditional compare and jump
instruction pair.
-The essential diff
Re: [PATCH] doc: update looping constructs
> On Jul 12, 2018, at 12:55 PM, Jeff Law wrote: > > On 07/11/2018 06:20 PM, Paul Koning wrote: >> This patch removes the obsolete documentation for >> decrement_and_branch_until_zero. It also adds detail to the description for >> doloop_end. In particular, it describes the required form of the >> conditional branch part of the pattern. >> >> Ok for trunk? >> >> paul >> >> ChangeLog: >> >> 2018-07-11 Paul Koning >> >> * doc/rtl.texi (REG_NONNEG): Remove decrement and branch until >> zero reference, add doloop_end instead. >> * doc/md.texi (decrement_and_branch_until_zero): Remove. >> (Looping patterns): Remove decrement_and_branch_until_zero. Add >> detail for doloop_end. > OK. I wonder if we can eliminate REG_NONNEG at this point. I'm not > sure if it's really being used anymore. I see a reference in the old > m68k dbra pattern, but that pattern could well be dead at this point. The original reasoning is that you'd need the note if you have a machine instruction that does a loop until negative, and you want to confirm that the input is a valid loop count (not negative -- otherwise you'd loop once rather than zero times). If you have a loop instruction that works this way, that still matters. If someone wants to convert m68000 to use doloop_end, for example. Or likewise for vax, which has an unnamed looping pattern that could be used but obviously isn't at the moment (since it's unnamed). Then again... does the code that generates this insn do the checking, i.e., avoid generating doloop_end unless it can confirm that the register is nonnegative? It doesn't seem to, at least not the code in "doloop_modify" that actually inserts the regnote. But how can that be valid? doloop_end by definition loops at least once. If you pass it a negative count, the loop is run once for "ge" comparisons, and MAXUINT + count times for insns that use the "ne" condition (i.e., all the current ones). So one would assume that's not possible, i.e., you couldn't come through there unless REG_NONNEG is guaranteed true. Currently the only doloop_end insns I can find all have the "ne" comparison so the regnote isn't generated. I'd like to leave this question separate from the doc cleanup. If we want to remove the note, that's easy enough, and the doc change would be pretty obvious as well. So is this patch OK to commit? paul
Re: [PATCH] doc: update looping constructs
> On Jul 12, 2018, at 1:42 PM, Jeff Law wrote: > > On 07/12/2018 11:17 AM, Paul Koning wrote: >> >> >>> On Jul 12, 2018, at 12:55 PM, Jeff Law wrote: >>> >>> On 07/11/2018 06:20 PM, Paul Koning wrote: >>>> This patch removes the obsolete documentation for >>>> decrement_and_branch_until_zero. It also adds detail to the >>>> description for doloop_end. In particular, it describes the >>>> required form of the conditional branch part of the pattern. >>>> >>>> Ok for trunk? >>>> >>>> paul >>>> >>>> ChangeLog: >>>> >>>> 2018-07-11 Paul Koning >>>> >>>> * doc/rtl.texi (REG_NONNEG): Remove decrement and branch until >>>> zero reference, add doloop_end instead. * doc/md.texi >>>> (decrement_and_branch_until_zero): Remove. (Looping patterns): >>>> Remove decrement_and_branch_until_zero. Add detail for >>>> doloop_end. >>> OK. I wonder if we can eliminate REG_NONNEG at this point. I'm >>> not sure if it's really being used anymore. I see a reference in >>> the old m68k dbra pattern, but that pattern could well be dead at >>> this point. >> >> The original reasoning is that you'd need the note if you have a >> machine instruction that does a loop until negative, and you want to >> confirm that the input is a valid loop count (not negative -- >> otherwise you'd loop once rather than zero times). If you have a >> loop instruction that works this way, that still matters. If someone >> wants to convert m68000 to use doloop_end, for example. Or likewise >> for vax, which has an unnamed looping pattern that could be used but >> obviously isn't at the moment (since it's unnamed). > Right. And that is now dbra works on the m68k. However I've got little > confidence we're generating that instruction anymore. Though I guess > it's easy enough to check since I can bootstrap m68k with qemu :-) > > It's always struck me as lame that we had to rely on the magic note and > in the world of doloop.c we ought to be able to express things much more > clearly. Agreed. I'm pretty sure dbra is no longer working. It certainly didn't do anything when I tried to add it in pdp11, and when I asked about it on the gcc list I was told that it was removed a decade ago, or thereabouts. The new technique works fine. The only drawback is the "register can't be the induction variable" limitation, which makes sense if special registers are used but would be nice not to have on machines where the register used is a general register. >> Then again... does the code that generates this insn do the checking, >> i.e., avoid generating doloop_end unless it can confirm that the >> register is nonnegative? It doesn't seem to, at least not the code >> in "doloop_modify" that actually inserts the regnote. But how can >> that be valid? doloop_end by definition loops at least once. If you >> pass it a negative count, the loop is run once for "ge" comparisons, >> and MAXUINT + count times for insns that use the "ne" condition >> (i.e., all the current ones). So one would assume that's not >> possible, i.e., you couldn't come through there unless REG_NONNEG is >> guaranteed true. >> >> Currently the only doloop_end insns I can find all have the "ne" >> comparison so the regnote isn't generated. I'd like to leave this >> question separate from the doc cleanup. If we want to remove the >> note, that's easy enough, and the doc change would be pretty obvious >> as well. >> >> So is this patch OK to commit? > SOrry I wasn't clear. I'm perfectly content to look at killing > REG_NONNEG independent of the doc cleanup. > > The doc patch is fine to commit now. Thanks, done. paul
[PATCH, committed] Fix typo in pdp11 target
This fixes a typo in the output of a ".set" directive.
Committed.
paul
ChangeLog:
2018-07-12 Paul Koning
* config/pdp11/pdp11.c (pdp11_output_def): Fix typo in .set
directive.
Index: config/pdp11/pdp11.c
===
--- config/pdp11/pdp11.c(revision 262602)
+++ config/pdp11/pdp11.c(working copy)
@@ -2437,7 +2437,7 @@ pdp11_output_def (FILE *file, const char *label1,
}
else
{
- fputs (".set", file);
+ fputs ("\t.set\t", file);
assemble_name (file, label1);
putc (',', file);
assemble_name (file, label2);
[PATCH, committed] Fix rtl check error in pdp11
When building with checking enabled, there were check failures in
pdp11_rtx_costs. This patch fixes two errors.
Committed.
paul
ChangeLog:
2018-07-14 Paul Koning
* config/pdp11/pdp11.c (pdp11_rtx_costs): Bugfixes.
Index: config/pdp11/pdp11.c
===
--- config/pdp11/pdp11.c(revision 262604)
+++ config/pdp11/pdp11.c(working copy)
@@ -1014,6 +1014,7 @@ pdp11_rtx_costs (rtx x, machine_mode mode, int out
const int code = GET_CODE (x);
const int asize = (mode == QImode) ? 2 : GET_MODE_SIZE (mode);
rtx src, dest;
+ const char *fmt;
switch (code)
{
@@ -1035,6 +1036,14 @@ pdp11_rtx_costs (rtx x, machine_mode mode, int out
*total = pdp11_addr_cost (x, mode, ADDR_SPACE_GENERIC, speed);
return true;
}
+ if (GET_RTX_LENGTH (code) == 0)
+{
+ if (speed)
+ *total = 0;
+ else
+ *total = 2;
+ return true;
+}
/* Pick up source and dest. We don't necessarily use the standard
recursion in rtx_costs to figure the cost, because that would
@@ -1041,6 +1050,15 @@ pdp11_rtx_costs (rtx x, machine_mode mode, int out
count the destination operand twice for three-operand insns.
Also, this way we can catch special cases like move of zero, or
add one. */
+ fmt = GET_RTX_FORMAT (code);
+ if (fmt[0] != 'e' || (GET_RTX_LENGTH (code) > 1 && fmt[1] != 'e'))
+{
+ if (speed)
+ *total = 0;
+ else
+ *total = 2;
+ return true;
+}
if (GET_RTX_LENGTH (code) > 1)
src = XEXP (x, 1);
dest = XEXP (x, 0);
Re: [2/5] C-SKY port: Backend implementation
> On Jul 25, 2018, at 12:50 AM, Jeff Law wrote: > ... >>> It did. See TARGET_CUSTOM_FUNCTION_DESCRIPTORS and the (relatively few) >>> ports that define it. >> >> Hmmm, I completely failed to make that connection from the docs -- the >> whole description of that hook is pretty gibberishy and I thought it was >> something for targets where the ABI already specifies some "standard >> calling sequence" using descriptors (C-SKY doesn't), rather than a >> generic alternative to executable trampolines. Putting on my doc >> maintainer hat briefly, I can see this needs a lot of work. :-( > Most likely :-) So many things to do, so little time. > > >> >> Anyway, is this required for new ports nowadays? If so, I at least know >> what to search for now. At this point I couldn't say whether this would >> do anything to fix the situation on ck801 targets where there simply >> aren't enough spare registers available to the trampoline to both hold >> the static link and do an indirect jump. > It's not required, but preferred, particularly if the part has an MMU > that can provide no-execute protections on pages in memory. If the > target doesn't have an mmu, then it's of marginal value. > > The key advantage it has over the old trampoline implementation is that > stacks can remain non-executable, even for Ada and nested functions. > That's a big win from a security standpoint. Non-executable stacks are a very good thing. That said, I also looked at the target hook documentation and was left without any clue whatsoever. It sure isn't clear what powers of two have to do with descriptors, or what descriptors have to do with support for nested functions. Can you suggest places to look to get an understanding of this feature? It sounds like the only other option is "Use the source, Luke". Any specific targets that make a good reference implementation for this? paul
Re: [PATCH 1/7] Add __builtin_speculation_safe_value
> On Jul 26, 2018, at 7:34 PM, Joseph Myers wrote: > > On Wed, 25 Jul 2018, Richard Earnshaw (lists) wrote: > Port maintainers DO need to decide what to do about speculation, even if it is explicitly that no mitigation is needed. >>> >>> Agreed. But I didn't yet see a request for maintainers to decide that? >>> >> >> consider it made, then :-) > > I suggest the following as an appropriate process for anything needing > attention from architecture maintainers: > > * Send a message to the gcc list, starting its own thread, CC:ed to all > target architecture maintainers, stating explicitly in its first sentence > that it is about something needing action from all such maintainers. Yes, because it was not clear to me that a patch discussion about a speculation builtin was something that every target maintainer was supposed to look at. "Speculation" is not a term that shows up in my target... > ... > * Over the next few months, send occasional reminders, each including a > list of the ports that have not been updated. Would the GCC Wiki be a good place to collect all the responses and track what is still open? If not, what is a good way to do the tracking? paul
Re: [PATCH 09/11] pdp11 - example of a port not needing a speculation barrier
> On Jul 27, 2018, at 5:37 AM, Richard Earnshaw > wrote: > > > This patch is intended as an example of all that is needed if the > target system doesn't support CPUs that have speculative execution. > I've chosen the pdp11 port on the basis that it's old enough that this > is likely to be true for all existing implementations and that there > is also little chance of that changing in future! > > * config/pdp11/pdp11.c (TARGET_HAVE_SPECULATION_SAFE_VALUE): Redefine > to speculation_safe_value_not_needed. > --- > gcc/config/pdp11/pdp11.c | 3 +++ > 1 file changed, 3 insertions(+) > > <0009-pdp11-example-of-a-port-not-needing-a-speculation-ba.patch> Correct, no speculative instruction now, and I don't think any of the people constructing PDP11s (yes there are some) are going to be adding it. Thanks Richard. paul
Re: [patch] improve internals documentation for nested function descriptors
> On Jul 27, 2018, at 4:39 PM, Sandra Loosemore wrote: > > Apropos of the discussion about improving the docs for > TARGET_CUSTOM_FUNCTION_DESCRIPTORS in the context of the C-SKY port > submission, > > https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01454.html > > here is the patch I've come up with based on reading the source. Is this > technically accurate? Any suggestions on how to improve it further? > > -Sandra > Nice. That tells me a lot more, so I hope to hear confirmation it's accurate. A nit: "word aligned" -- you used thas as a synonym for "multiple of 4 aligned" but some of us have 2-byte words. On the function descriptor concept generally: it wasn't clear to me whether a target that wants to use this mechanism needs to do anything to tell GCC how to construct descriptors. Does GCC have a standard recipe it uses that "should" work for any target? I assume so because there aren't any other target hooks or special patterns for this. It would be helpful to say so explicitly. paul
Re: Deprecating cc0 (and consequently cc0 targets)
> On Oct 30, 2019, at 2:24 PM, Jeff Law wrote:
>
> On 10/30/19 2:12 AM, Richard Biener wrote:
>> On Tue, Oct 29, 2019 at 8:34 PM Jeff Law wrote:
>
>>
>> I think the wiki has better examples. That said, I wonder how much can
>> be automated here, for example when just considering CCmode (m68k has
>> setcc IIRC) then for example all define_insns like
>>
>> (define_insn "*cmpsi_cf"
>> [(set (cc0)
>>(compare (match_operand:SI 0 "nonimmediate_operand" "mrKs,r")
>> (match_operand:SI 1 "general_operand" "r,mrKs")))]
>> "TARGET_COLDFIRE"
>> {...}
> The compare/test insns are relatively straightforward as they're a
> fairly simple substitution. It's all the other insns that implicitly
> set cc0 that are so painful.
>
>
>>
>> The various expanders need to be handled manually I guess, though somehow
>> automagically generating the define_insn_and_split might be possible
>> as well, no?
> I was looking at define_subst to help here, but it didn't look like it
> was really going to simplify things in any significant way. Maybe
> someone with more experience with define_subst would see something that
> would dramtically simplify this process.
I've found define_subst to be quite helpful for generating the pairs of
"clobbers CC" and "sets CC in a useful way from the operation result" pairs
that Richard was referring to. pdp11.md shows what I learned; it may be
applicable elsewhere too. This is for a "case 2" target, one that modifies CC
on most operations (essentially all those that do arithmetic).
paul
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
> On Dec 5, 2019, at 11:17 AM, Joseph Myers wrote: > > On Thu, 5 Dec 2019, Thomas Schwinge wrote: > >> In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner >> stated that even he is not using a 80 x 24 terminal anymore, and that >> should tell us something. ;-) >> >> So, I formally propose that we lift this characters per line restriction >> from IBM punch card (80) to mainframe line printer (132). > > I thought these line lengths were based on readability studies suggesting > lengths that lines shorter than 80 columns were more readable? That's certainly a general rule. There is a reason why books aren't wide, and why newspapers have columns. The eye can't deal well with long lines. So while 132 column lines are certainly possible with modern computers, it doesn't mean they are desirable. paul
Re: [PATCH] Turn on LRA on all targets
> On May 15, 2023, at 5:09 PM, Maciej W. Rozycki wrote: > > ... > > I may choose to implement a non-DWARF unwinder instead, as the VAX stack > frame is always fully described by the hardware and there is never ever a > need for debug information to be able to decode any VAX stack frame (the > RET machine instruction uses the stack frame information to restore the > previous PC, FP, SP, AP and any static registers saved by CALLS). That would make sense; it's like the heuristic unwinder found in some other targets (I remember the MIPS one, which worked fairly well and allowed debugging without useable debug data). paul
Re: [PATCH] Turn on LRA on all targets
> On Feb 15, 2024, at 5:56 PM, Segher Boessenkool > wrote: > > On Thu, Feb 15, 2024 at 07:34:32PM +, Sam James wrote: >> I have now started doing this in PR113932. > > Thank you! > > Segher Presumably this isn't for version 14 since it's in a late stage, right? I have my bits about ready to go in but I'll wait for State 1 to open. Correct? paul
Re: [PATCH] Turn on LRA on all targets
> On Feb 16, 2024, at 6:34 AM, Maciej W. Rozycki wrote: > > On Thu, 15 Feb 2024, Paul Koning wrote: > >>> On May 15, 2023, at 5:09 PM, Maciej W. Rozycki wrote: >>> >>> ... >>> >>> I may choose to implement a non-DWARF unwinder instead, as the VAX stack >>> frame is always fully described by the hardware and there is never ever a >>> need for debug information to be able to decode any VAX stack frame (the >>> RET machine instruction uses the stack frame information to restore the >>> previous PC, FP, SP, AP and any static registers saved by CALLS). >> >> That would make sense; it's like the heuristic unwinder found in some >> other targets (I remember the MIPS one, which worked fairly well and >> allowed debugging without useable debug data). > > Not really, in particular because EH unwinding has to be reliable and > heuristics inherently is not. Fair enough, but what I meant is only that it's conceptually similar: unwind based on the code and machine state, rather than auxiliary information like debug data. And I think your point was that on VAX this is indeed a reliable technique by virtue of the iSA. In fact, the standard way to do exeception handling unwinding is part of the originail VAX architecture (via the otherwise unused first word (I think) of the call frame). paul
Re: [PATCH] Fix PR ada/111909 On Darwin, determine filesystem case sensitivity at runtime
> On Nov 22, 2023, at 8:54 AM, Simon Wright wrote: > > On 21 Nov 2023, at 23:13, Iain Sandoe wrote: > >>> #if defined (__APPLE__) >>> -#include >> >> If removing unistd.h is intentional (i.e. you determined that it’s no longer >> needed for Darwin), then we should make that a separate patch. > > I thought that I’d had to include unistd.h for the first patch in this > thread; clearly not! > > What I hope will be the final version: > > ——— 8< .——— > > In gcc/ada/adaint.c(__gnat_get_file_names_case_sensitive), the current > assumption for __APPLE__ is that file names are case-insensitive > unless __arm__ or __arm64__ are defined, in which case file names are > declared case-sensitive. > > The associated comment is > "By default, we suppose filesystems aren't case sensitive on > Windows and Darwin (but they are on arm-darwin)." > > This means that on aarch64-apple-darwin, file names are treated as > case-sensitive, which is not the default case. > > The true default position is that macOS file systems are > case-insensitive, iOS file systems are case-sensitive. Sort of. The most common choices for Mac OS file system type are indeed case insensitive, but it also allows case sensitive file systems. paul
Re: [committed] PATCH for Re: Stepping down as maintainer for ARC and Epiphany
> On May 21, 2024, at 9:57 AM, Jeff Law wrote: > > > > On 5/21/24 12:05 AM, Richard Biener via Gcc wrote: >> On Mon, May 20, 2024 at 4:45 PM Gerald Pfeifer wrote: >>> >>> On Wed, 5 Jul 2023, Joern Rennecke wrote: I haven't worked with these targets in years and can't really do sensible maintenance or reviews of patches for them. I am currently working on optimizations for other ports like RISC-V. >>> >>> I noticed MAINTAINERS was not updated, so pushed the patch below. >> That leaves the epiphany port unmaintained. Should we automatically add such >> ports to the list of obsoleted ports? > Given that epiphany has randomly failed tests for the last 3+ years due to > bugs in its patterns, yes, it really needs to be deprecated. > > I tried to fix the worst of the offenders in epiphany.md a few years back and > gave up. Essentially seemingly innocent changes in the RTL will cause reload > to occasionally not see a path to get constraints satisfied. So a test which > passes today, will flip to failing tomorrow while some other test of tests > will go the other way. Does LRA make that issue go away, or does it not help? paul
Re: LRA: Fix setup_sp_offset
> On Aug 26, 2024, at 10:14 AM, Michael Matz wrote:
>
> Hello,
>
> On Sun, 25 Aug 2024, Jeff Law wrote:
>
>>> 550: [--sp] = 0 sp_off = 0 {pushexthisi_const}
>>> 551: [--sp] = 37sp_off = -4 {pushexthisi_const}
>>> 552: [--sp] = r37 sp_off = -8 {movsi_m68k2}
>>> 554: [--sp] = r116 - r37sp_off = -12 {subsi3}
>>> 556: call sp_off = -16
>>>
>>> insn 554 doesn't match its constraints and needs some reloads:
>>
>> I think you're right in that the current code isn't correct, but the
>> natural question is how in the world has this worked to-date. Though I
>> guess targets which push arguments are a dying breed (though I would
>> have expected i386 to have tripped over this at some point).
>
> Yeah, I wondered as well. For things to go wrong some instructions that
> contain pre/post-inc/dec of the stack pointer need to have reloads in such
> a way that the actual SP-change sideeffect moves to a different
> instruction.
I think I've seen that in the past on PDP11, and reported it, but I thought
that particular issue was fixed not too long after.
paul
Re: LRA: Fix setup_sp_offset
> On Aug 26, 2024, at 10:40 AM, Michael Matz wrote:
>
> Hello,
>
> On Mon, 26 Aug 2024, Paul Koning wrote:
>
>>>>> 550: [--sp] = 0 sp_off = 0 {pushexthisi_const}
>>>>> 551: [--sp] = 37sp_off = -4 {pushexthisi_const}
>>>>> 552: [--sp] = r37 sp_off = -8 {movsi_m68k2}
>>>>> 554: [--sp] = r116 - r37sp_off = -12 {subsi3}
>>>>> 556: call sp_off = -16
>>>>>
>>>>> insn 554 doesn't match its constraints and needs some reloads:
>>>>
>>>> I think you're right in that the current code isn't correct, but the
>>>> natural question is how in the world has this worked to-date. Though I
>>>> guess targets which push arguments are a dying breed (though I would
>>>> have expected i386 to have tripped over this at some point).
>>>
>>> Yeah, I wondered as well. For things to go wrong some instructions that
>>> contain pre/post-inc/dec of the stack pointer need to have reloads in such
>>> a way that the actual SP-change sideeffect moves to a different
>>> instruction.
>>
>> I think I've seen that in the past on PDP11, and reported it, but I
>> thought that particular issue was fixed not too long after.
>
> Do you have a reference handy? I'd like to take a look, if for nothing
> else than curiosity ;-)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87944
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87944> which says it was fixed in
GCC 14 on 5/30/2023.
paul
Re: PING^1 [PATCH 30/52] pdp11: Remove macro LONG_DOUBLE_TYPE_SIZE
What is the effect of this change? The original code intended to have "float" mean a 32 bit value, and "double" a 64 bit value. There aren't any larger floats, so I defined the long double size as 64 also. Is the right answer not to define it? That part I understand, but why does the patch also remove FLOAT_TYPE_SIZE and DOUBLE_TYPE_SIZE without explanation and without mention in the changelog? paul > On Jun 13, 2024, at 3:16 AM, Kewen.Lin wrote: > > Hi, > > Gentle ping: > > https://gcc.gnu.org/pipermail/gcc-patches/2024-June/653368.html > > BR, > Kewen > > on 2024/6/3 11:01, Kewen Lin wrote: >> This is to remove macro LONG_DOUBLE_TYPE_SIZE define >> in pdp11 port. >> >> gcc/ChangeLog: >> >> * config/pdp11/pdp11.h (LONG_DOUBLE_TYPE_SIZE): Remove. >> --- >> gcc/config/pdp11/pdp11.h | 11 --- >> 1 file changed, 11 deletions(-) >> >> diff --git a/gcc/config/pdp11/pdp11.h b/gcc/config/pdp11/pdp11.h >> index 2446fea0b58..6c8e045bc57 100644 >> --- a/gcc/config/pdp11/pdp11.h >> +++ b/gcc/config/pdp11/pdp11.h >> @@ -71,17 +71,6 @@ along with GCC; see the file COPYING3. If not see >> #define LONG_TYPE_SIZE 32 >> #define LONG_LONG_TYPE_SIZE 64 >> >> -/* In earlier versions, FLOAT_TYPE_SIZE was selectable as 32 or 64, >> - but that conflicts with Fortran language rules. Since there is no >> - obvious reason why we should have that feature -- other targets >> - generally don't have float and double the same size -- I've removed >> - it. Note that it continues to be true (for now) that arithmetic is >> - always done with 64-bit values, i.e., the FPU is always in "double" >> - mode. */ >> -#define FLOAT_TYPE_SIZE 32 >> -#define DOUBLE_TYPE_SIZE64 >> -#define LONG_DOUBLE_TYPE_SIZE 64 >> - >> /* machine types from ansi */ >> #define SIZE_TYPE "short unsigned int" /* definition of size_t */ >> #define WCHAR_TYPE "short int" /* or long int */ > > >
Re: [PATCH 30/52 v2] pdp11: Remove macro {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
Ok, I understand better now. But if those macros are supposed to be replaced
by hook functions, could you make that replacement part of the proposed patch?
paul
> On Jun 13, 2024, at 11:22 PM, Kewen.Lin wrote:
>
> Hi Paul,
>
> on 2024/6/14 04:07, Paul Koning wrote:
>> What is the effect of this change? The original code intended to have
>> "float" mean a 32 bit value, and "double" a 64 bit value. There aren't any
>> larger floats, so I defined the long double size as 64 also. Is the right
>> answer not to define it?
>
> Since sub-patch 09/52 will poison {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE,
> target code building will fail
> if it still has these macros. As I'd like to squash these target changes
> onto 09/52, so I didn't note
> the background/context here, sorry about that.
>
>>
>> That part I understand, but why does the patch also remove FLOAT_TYPE_SIZE
>> and DOUBLE_TYPE_SIZE without explanation and without mention in the
>> changelog?
>
> Oops, thanks for catching! I just noticed this sub-patch has inconsistent
> subject & changelog, I should
> have noticed this as it has a quite different subject from the others. :(
> With your finding, I just
> re-visited all the other sub-patches, luckily they are consistent.
>
> The below is the updated revision, hope it looks good to you. Thanks again.
>
> BR,
> Kewen
> -
>
> Subject: [PATCH] pdp11: Remove macro {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
>
> This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE
> defines in pdp11 port, as we want to replace these macros
> with hook mode_for_floating_type and poison them.
>
> gcc/ChangeLog:
>
>* config/pdp11/pdp11.h (FLOAT_TYPE_SIZE): Remove.
>(DOUBLE_TYPE_SIZE): Likewise.
>(LONG_DOUBLE_TYPE_SIZE): Likewise.
> ---
> gcc/config/pdp11/pdp11.h | 11 ---
> 1 file changed, 11 deletions(-)
>
> diff --git a/gcc/config/pdp11/pdp11.h b/gcc/config/pdp11/pdp11.h
> index 2446fea0b58..6c8e045bc57 100644
> --- a/gcc/config/pdp11/pdp11.h
> +++ b/gcc/config/pdp11/pdp11.h
> @@ -71,17 +71,6 @@ along with GCC; see the file COPYING3. If not see
> #define LONG_TYPE_SIZE 32
> #define LONG_LONG_TYPE_SIZE64
>
> -/* In earlier versions, FLOAT_TYPE_SIZE was selectable as 32 or 64,
> - but that conflicts with Fortran language rules. Since there is no
> - obvious reason why we should have that feature -- other targets
> - generally don't have float and double the same size -- I've removed
> - it. Note that it continues to be true (for now) that arithmetic is
> - always done with 64-bit values, i.e., the FPU is always in "double"
> - mode. */
> -#define FLOAT_TYPE_SIZE32
> -#define DOUBLE_TYPE_SIZE 64
> -#define LONG_DOUBLE_TYPE_SIZE 64
> -
> /* machine types from ansi */
> #define SIZE_TYPE "short unsigned int" /* definition of size_t */
> #define WCHAR_TYPE "short int" /* or long int */
> --
> 2.43.0
>
>
Re: [PATCH 30/52 v2] pdp11: Remove macro {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
Thanks Kewen.
Given that background, the patch is OK.
paul
> On Jun 16, 2024, at 10:01 PM, Kewen.Lin wrote:
>
> Hi Paul,
>
> on 2024/6/14 23:20, Paul Koning wrote:
>> Ok, I understand better now. But if those macros are supposed to be
>> replaced by hook functions, could you make that replacement part of the
>> proposed patch?
>
> The default implementation of the introduced hook mode_for_floating_type
> returns SFmode for float and DFmode for double or long double, which matches
> what pdp11 port requires, so there is no need to add its own hook
> implementation.
> This patch series only re-define this hook macro with the customized hook
> implementation for those ports which need something beyond the default.
>
> BR,
> Kewen
>
>>
>> paul
>>
>>> On Jun 13, 2024, at 11:22 PM, Kewen.Lin wrote:
>>>
>>> Hi Paul,
>>>
>>> on 2024/6/14 04:07, Paul Koning wrote:
>>>> What is the effect of this change? The original code intended to have
>>>> "float" mean a 32 bit value, and "double" a 64 bit value. There aren't
>>>> any larger floats, so I defined the long double size as 64 also. Is the
>>>> right answer not to define it?
>>>
>>> Since sub-patch 09/52 will poison {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE,
>>> target code building will fail
>>> if it still has these macros. As I'd like to squash these target changes
>>> onto 09/52, so I didn't note
>>> the background/context here, sorry about that.
>>>
>>>>
>>>> That part I understand, but why does the patch also remove FLOAT_TYPE_SIZE
>>>> and DOUBLE_TYPE_SIZE without explanation and without mention in the
>>>> changelog?
>>>
>>> Oops, thanks for catching! I just noticed this sub-patch has inconsistent
>>> subject & changelog, I should
>>> have noticed this as it has a quite different subject from the others. :(
>>> With your finding, I just
>>> re-visited all the other sub-patches, luckily they are consistent.
>>>
>>> The below is the updated revision, hope it looks good to you. Thanks again.
>>>
>>> BR,
>>> Kewen
>>> -
>>>
>>> Subject: [PATCH] pdp11: Remove macro {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE
>>>
>>> This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE
>>> defines in pdp11 port, as we want to replace these macros
>>> with hook mode_for_floating_type and poison them.
>>>
>>> gcc/ChangeLog:
>>>
>>> * config/pdp11/pdp11.h (FLOAT_TYPE_SIZE): Remove.
>>> (DOUBLE_TYPE_SIZE): Likewise.
>>> (LONG_DOUBLE_TYPE_SIZE): Likewise.
>>> ---
>>> gcc/config/pdp11/pdp11.h | 11 ---
>>> 1 file changed, 11 deletions(-)
>>>
>>> diff --git a/gcc/config/pdp11/pdp11.h b/gcc/config/pdp11/pdp11.h
>>> index 2446fea0b58..6c8e045bc57 100644
>>> --- a/gcc/config/pdp11/pdp11.h
>>> +++ b/gcc/config/pdp11/pdp11.h
>>> @@ -71,17 +71,6 @@ along with GCC; see the file COPYING3. If not see
>>> #define LONG_TYPE_SIZE 32
>>> #define LONG_LONG_TYPE_SIZE64
>>>
>>> -/* In earlier versions, FLOAT_TYPE_SIZE was selectable as 32 or 64,
>>> - but that conflicts with Fortran language rules. Since there is no
>>> - obvious reason why we should have that feature -- other targets
>>> - generally don't have float and double the same size -- I've removed
>>> - it. Note that it continues to be true (for now) that arithmetic is
>>> - always done with 64-bit values, i.e., the FPU is always in "double"
>>> - mode. */
>>> -#define FLOAT_TYPE_SIZE32
>>> -#define DOUBLE_TYPE_SIZE 64
>>> -#define LONG_DOUBLE_TYPE_SIZE 64
>>> -
>>> /* machine types from ansi */
>>> #define SIZE_TYPE "short unsigned int" /* definition of size_t */
>>> #define WCHAR_TYPE "short int" /* or long int */
>>> --
>>> 2.43.0
>>>
>>>
>>
>
Re: [PATCH] Hard register asm constraint
> On Jun 24, 2024, at 1:50 AM, Stefan Schulze Frielinghaus
> wrote:
>
> Ping.
>
> On Mon, Jun 10, 2024 at 07:19:19AM +0200, Stefan Schulze Frielinghaus wrote:
>> Ping.
>>
>> On Fri, May 24, 2024 at 11:13:12AM +0200, Stefan Schulze Frielinghaus wrote:
>>> This implements hard register constraints for inline asm. A hard register
>>> constraint is of the form {regname} where regname is any valid register.
>>> This
>>> basically renders register asm superfluous. For example, the snippet
>>>
>>> int test (int x, int y)
>>> {
>>> register int r4 asm ("r4") = x;
>>> register int r5 asm ("r5") = y;
>>> unsigned int copy = y;
>>> asm ("foo %0,%1,%2" : "+d" (r4) : "d" (r5), "d" (copy));
>>> return r4;
>>> }
>>>
>>> could be rewritten into
>>>
>>> int test (int x, int y)
>>> {
>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
>>> return x;
>>> }
I like this idea but I'm wondering: regular constraints specify what sort of
value is needed, for example an int vs. a short int vs. a float. The notation
you've shown doesn't seem to have that aspect.
The other comment is that I didn't see documentation updates to reflect this
new feature.
paul
Re: [PATCH] Hard register asm constraint
> On Jun 25, 2024, at 12:04 PM, Stefan Schulze Frielinghaus
> wrote:
>
> On Tue, Jun 25, 2024 at 10:03:34AM -0400, Paul Koning wrote:
>>
>>>>> ...
>>>>> could be rewritten into
>>>>>
>>>>> int test (int x, int y)
>>>>> {
>>>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
>>>>> return x;
>>>>> }
>>
>> I like this idea but I'm wondering: regular constraints specify what sort of
>> value is needed, for example an int vs. a short int vs. a float. The
>> notation you've shown doesn't seem to have that aspect.
>
> As Maciej already pointed out the type of the expression should suffice.
> My assumption was that an asm can deal with a value as is or its
> promoted value. At least for integer values this should be fine and
> AFAICS is also the case for simple constraints like "r" which do not
> define any mode. I've probably overseen something but which constraint
> differentiates between int vs short? However, you have a good point
> with this and I should test this more.
I thought there was but I may be confused. On the other hand, there definitely
are (machine dependent) constraints that distinguish, say, float from integer
registers; pdp11 is an example. If you were to use an "a" constraint, that
means a floating point register and the compiler will detect attempts to pass
non-float operands ("Inconsistent operand constraints...").
I see that the existing "register int ..." syntax appears to check that the
register is the right type for the data type given for it, so for example on
pdp11,
register int ac1 asm ("ac1") = i;
fails ("register ... isn't suitable for data type"). I assume your new syntax
would perform the same check and produce roughly the same error message. You
might verify that. On pdp11, trying to use, for example, "r0" for a float, or
"ac0" for an int, would produce that error.
With all that, I think your approach does work right and the question I raised
isn't actually a problem.
>> The other comment is that I didn't see documentation updates to reflect this
>> new feature.
>
> I didn't came up with documentation yet since I was not sure whether
> such a proposal would be accepted at all, i.e., just wanted to hear
> whether you see some show stoppers or not. Assuming this goes well I
> guess it should be documented under simple constraints
> https://gcc.gnu.org/onlinedocs/gcc/Simple-Constraints.html
>
> Thanks,
> Stefan
That seems right. I think it would be good for one of the global maintainers,
or people of similar stature, to say whether Stefan's proposal is one that
should be considered. My 2c worth is that it should be.
The documentation might want to mention that the compiler will confirm that the
register specified is suitable for the type given to it, just as it does for
the old syntax.
paul
Re: [PATCH] Hard register asm constraint
> On Jun 26, 2024, at 8:54 AM, Stefan Schulze Frielinghaus
> wrote:
>
> On Tue, Jun 25, 2024 at 01:02:39PM -0400, Paul Koning wrote:
>>
>>
>>> On Jun 25, 2024, at 12:04 PM, Stefan Schulze Frielinghaus
>>> wrote:
>>>
>>> On Tue, Jun 25, 2024 at 10:03:34AM -0400, Paul Koning wrote:
>>>>
>>>>>>> ...
>>>>>>> could be rewritten into
>>>>>>>
>>>>>>> int test (int x, int y)
>>>>>>> {
>>>>>>> asm ("foo %0,%1,%2" : "+{r4}" (x) : "{r5}" (y), "d" (y));
>>>>>>> return x;
>>>>>>> }
>>>>
>>>> I like this idea but I'm wondering: regular constraints specify what sort
>>>> of value is needed, for example an int vs. a short int vs. a float. The
>>>> notation you've shown doesn't seem to have that aspect.
>>>
>>> As Maciej already pointed out the type of the expression should suffice.
>>> My assumption was that an asm can deal with a value as is or its
>>> promoted value. At least for integer values this should be fine and
>>> AFAICS is also the case for simple constraints like "r" which do not
>>> define any mode. I've probably overseen something but which constraint
>>> differentiates between int vs short? However, you have a good point
>>> with this and I should test this more.
>>
>> I thought there was but I may be confused. On the other hand, there
>> definitely are (machine dependent) constraints that distinguish, say, float
>> from integer registers; pdp11 is an example. If you were to use an "a"
>> constraint, that means a floating point register and the compiler will
>> detect attempts to pass non-float operands ("Inconsistent operand
>> constraints...").
>>
>> I see that the existing "register int ..." syntax appears to check that the
>> register is the right type for the data type given for it, so for example on
>> pdp11,
>>
>> register int ac1 asm ("ac1") = i;
>>
>> fails ("register ... isn't suitable for data type"). I assume your new
>> syntax would perform the same check and produce roughly the same error
>> message. You might verify that. On pdp11, trying to use, for example, "r0"
>> for a float, or "ac0" for an int, would produce that error.
>
> Right, so far I don't error out here which I will change. It basically
> results in bit casting floats to ints currently.
That would be bad. For one thing, a PDP11 float doesn't fit in an integer
register.
That also brings up another point (which applies to more mainstream targets as
well): for data types that require multiple registers, say a register pair for
a double length value, how is that handled? One possible answer is to reject
that. Another would be to load a register pair.
This case applies to a "long int" on pdp11, or 32 bit MIPS, and probably a
bunch of others.
paul
Re: [PATCH 1/3][v2] Add TARGET_MODE_CAN_TRANSFER_BITS
> On Jul 30, 2024, at 6:17 AM, Richard Biener wrote:
>
> The following adds a target hook to specify whether regs of MODE can be
> used to transfer bits. The hook is supposed to be used for value-numbering
> to decide whether a value loaded in such mode can be punned to another
> mode instead of re-loading the value in the other mode and for SRA to
> decide whether MODE is suitable as container holding a value to be
> used in different modes.
>
> ...
>
> +@deftypefn {Target Hook} bool TARGET_MODE_CAN_TRANSFER_BITS (machine_mode
> @var{mode})
> +Define this to return false if the mode @var{mode} cannot be used
> +for memory copying. The default is to assume modes with the same
> +precision as size are fine to be used.
> +@end deftypefn
> +
I'm a bit confused about the meaning of this hook; the summary at the top
speaks of type punning while the documentation talks about memory copying.
Those seem rather different.
I'm also wondering about this being tied to a mode rather than a register
class. To given an example: on the PDP11 there are two main register classes,
"general" and "float". General registers handle any bit pattern and support
arithmetic operations on integer modes; float registers do not transparently
transfer every bit pattern and support float modes. So only general registers
are suitable for memory copies (though on a PDP-11 you don't need registers to
do memory copy). And for type punning, you could load an SF mode value into
general registers (a pair) and type-pun them to SImode without reloading.
So what does that mean for this hook on that target?
paul
Re: Problem exposed by recent ARM multiply changes
> On Sep 26, 2019, at 12:01 PM, Jeff Law wrote: > > On 9/26/19 9:47 AM, Jakub Jelinek wrote: >> On Thu, Sep 26, 2019 at 09:39:31AM -0600, Jeff Law wrote: >>> Right. My point is that the multiplication patterns are an exception as >>> well. >> >> Do you have some evidence for that? > It's in the manual. And yes it potentially makes a huge mess due to the > interactions with modeless CONST_INTs. Do you mean things like "mulhisi3"? That's a pattern for a multiplication whose output is longer than its inputs. But the RTL it builds has a regular mult insn with equal length operands (all SImode in this example), with sign_extend insns in the mult source operands. So I don't believe that contradicts what Jakub said. paul
Re: [PATCH] regrename: Use PC instead of CC0 to hide operands
On Oct 1, 2019, at 5:14 AM, Segher Boessenkool wrote: > > The regrename pass temporarily changes some operand RTL to CC0 so that > note_stores and scan_rtx don't see those operands. CC0 is deprecated > and we want to remove it, so we need to use something else here. > PC fits the bill fine. CC0 is, presumably, not part of GENERAL_REGS, but PC is, in some ports. Does that cause a problem here? paul
Re: Fix ALL_REGS thinko in initialisation of function_used_regs
> On Oct 3, 2019, at 9:12 AM, Richard Earnshaw (lists) > wrote: > > On 03/10/2019 10:48, Segher Boessenkool wrote: >>> ... >> But ALL_REGS should contain *all* registers. The union of any two register >> classes has to be contained in some register class, so every register class >> has to be contained in ALL_REGS. >> Segher > > Why would anyone want a to form a union of one class with a class that can't > be used for allocation. That's just silly. Yes, if that's the only purpose of register classes. If the documentation doesn't match what the real intent is, and some developers apparently use the intent while others use the documentation. The result is bugs like the one we're talking about here. It may well make sense to say that ALL_REGS should be the set of all *allocatable* registers. If that's what we want, the documentation should say so, and then the code has to understand that some registers (like CC_REG) may be outside ALL_REGS. Similarly, NO_REGS is currently documented as "no registers" (empty set). If it is intended that it might contain non-allocatable registers, again the documentation should say so. paul
Re: [PATCH 3/4] Set costs for jumps in combine
> On Nov 21, 2019, at 7:42 PM, Segher Boessenkool > wrote: > > ... > Maybe i386 should implement the insn_cost hook as well? For most targets > that is a lot simpler to get right than rtx_cost, but allowing memory in > many insns and all the different insn lengths complicates matters. At > least insn_cost isn't inside-out, that should make it easier to deal with > already. Yes, rtx_cost is a pain to write and understand. I looked at insn_cost in the past but was told, or got the impression somehow, that it is at the moment a partial solution and rtx_cost is still needed to complete the cost picture. Is that incorrect, or no longer accurate? I would like to dump rtx_cost in my target if that is now indeed a valid thing to do. paul
Re: [PATCH] Use getentropy() for seeding PRNG
> On Aug 3, 2018, at 9:19 AM, Janne Blomqvist wrote: > > The getentropy function, found on Linux, OpenBSD, and recently also > FreeBSD, can be used to get random bytes to initialize the PRNG. It > is similar to the traditional way of reading from /dev/urandom, but > being a system call rather than a special file, it doesn't suffer from > problems like running out of file descriptors, or failure when running > in a container where /dev/urandom is not available. I don't understand why this is useful. getrandom, and /dev/random, are for strong (secure) RNGs. A PRNG is something entirely different. By saying we use entropy to seed it, we blur the distinction and create the false impression that the PRNG has security properties. It would be better to initialize with something more obviously insecure, like gettimeofday(). paul
Re: [PATCH] Add sinh(tanh(x)) and cosh(tanh(x)) rules
> On Aug 7, 2018, at 4:00 PM, Giuliano Augusto Faulin Belinassi > wrote: > > Related with bug 86829, but for hyperbolic trigonometric functions. > This patch adds substitution rules to both sinh(tanh(x)) -> x / sqrt(1 > - x*x) and cosh(tanh(x)) -> 1 / sqrt(1 - x*x). Notice that the both > formulas has division by 0, but it causes no harm because 1/(+0) -> > +infinity, thus the math is still safe. What about non-IEEE targets that don't have "infinite" in their float representation? paul
Re: [PATCH] Add sinh(tanh(x)) and cosh(tanh(x)) rules
Now I'm puzzled. I don't see how an infinite would show up in the original expression. I don't know hyperbolic functions, so I just constructed a small test program, and the original vs. the substitution you mention are not at all similar. paul > On Aug 7, 2018, at 4:42 PM, Giuliano Augusto Faulin Belinassi > wrote: > > That is a good question because I didn't know that such targets > exists. Any suggestion? > > > On Tue, Aug 7, 2018 at 5:29 PM, Paul Koning wrote: >> >> >>> On Aug 7, 2018, at 4:00 PM, Giuliano Augusto Faulin Belinassi >>> wrote: >>> >>> Related with bug 86829, but for hyperbolic trigonometric functions. >>> This patch adds substitution rules to both sinh(tanh(x)) -> x / sqrt(1 >>> - x*x) and cosh(tanh(x)) -> 1 / sqrt(1 - x*x). Notice that the both >>> formulas has division by 0, but it causes no harm because 1/(+0) -> >>> +infinity, thus the math is still safe. >> >> What about non-IEEE targets that don't have "infinite" in their float >> representation? >> >>paul >> >>
Re: [PATCH] Add sinh(tanh(x)) and cosh(tanh(x)) rules
Thanks. Ok, so the expressions you gave are undefined for x==1, which says that substituting something that is also undefined for x==1 is permitted. You can argue from "undefined" rather than relying on IEEE features like NaN or infinite. paul > On Aug 8, 2018, at 2:57 PM, Giuliano Augusto Faulin Belinassi > wrote: > > Sorry about that. In the e-mail text field I wrote sinh(tanh(x)) and > cosh(tanh(x)) where it was supposed to be sinh(atanh(x)) and > cosh(atanh(x)), thus I am talking about the inverse hyperbolic tangent > function. The patch code and comments are still correct. > > On Wed, Aug 8, 2018 at 10:58 AM, Paul Koning wrote: >> Now I'm puzzled. >> >> I don't see how an infinite would show up in the original expression. I >> don't know hyperbolic functions, so I just constructed a small test program, >> and the original vs. the substitution you mention are not at all similar. >> >>paul >> >> >>> On Aug 7, 2018, at 4:42 PM, Giuliano Augusto Faulin Belinassi >>> wrote: >>> >>> That is a good question because I didn't know that such targets >>> exists. Any suggestion? >>> >>> >>> On Tue, Aug 7, 2018 at 5:29 PM, Paul Koning wrote: >>>> >>>> >>>>> On Aug 7, 2018, at 4:00 PM, Giuliano Augusto Faulin Belinassi >>>>> wrote: >>>>> >>>>> Related with bug 86829, but for hyperbolic trigonometric functions. >>>>> This patch adds substitution rules to both sinh(tanh(x)) -> x / sqrt(1 >>>>> - x*x) and cosh(tanh(x)) -> 1 / sqrt(1 - x*x). Notice that the both >>>>> formulas has division by 0, but it causes no harm because 1/(+0) -> >>>>> +infinity, thus the math is still safe. >>>> >>>> What about non-IEEE targets that don't have "infinite" in their float >>>> representation? >>>> >>>> paul >>>> >>>> >>
Re: [PATCH 3/3] or1k: gcc: initial support for openrisc
> On Aug 30, 2018, at 9:02 PM, Jeff Law wrote: > > On 08/30/2018 10:58 AM, Richard Henderson wrote: >> On 08/28/2018 07:13 AM, Jeff Law wrote: >>> Please consider using function descriptors rather than trampolines. >>> This allows you to make the stack non-executable at all times which is >>> good from a security standpoint. The downside is the indirect calling >>> mechanism has to change slightly to distinguish between a simple >>> indirect call and one through a function descriptor (usually by having a >>> low bit on to indicate the latter). GIven this is an ABI change, now is >>> probably the last opportunity to make this change. >> >> Correct me if I'm wrong here: >> >> Define TARGET_CUSTOM_FUNCTION_DESCRIPTORS to an appropriate value -- easy >> for a >> RISC target -- and that's it. >> >> Further, it pretty much only gets used by the Ada front end. One should not >> expect these to be used by the C front end nested functions. > I thought it was used more extensively than that... Thanks for checking > into it though. > > Jeff My impression from reading the internals manual is that it's an alternative to trampolines -- and in fact it appears to suggest it's a superior alternative. I've been planning to try turning it on for pdp11 where executable stacks can be problematic. (For that matter, they are on lots of other machines -- which is why descriptors instead of trampolines sounds like a good thing.) paul
[PATCH, committed] fix a number of pdp11 target test failures
This fixes a number of test failures due to test cases that are too large for pdp11 and should be skipped. Also one test case that asks for alignment larger than what pdp11 supports. paul 2018-05-25 Paul Koning * gcc.c-torture/compile/20151204.c: Skip if pdp11. * gcc.c-torture/compile/pr55921.c: Ditto. * gcc.c-torture/compile/pr60655-1.c: Ditto. * gcc.c-torture/compile/vector-align-1.c: Add max alignment if pdp11.
[PATCH, committed] fix a number of pdp11 target test failures
Sorry, it's been a while, previous message was incomplete.
This fixes a number of test failures due to test cases
that are too large for pdp11 and should be skipped. Also
one test case that asks for alignment larger than what
pdp11 supports.
paul
2018-05-25 Paul Koning
* gcc.c-torture/compile/20151204.c: Skip if pdp11.
* gcc.c-torture/compile/pr55921.c: Ditto.
* gcc.c-torture/compile/pr60655-1.c: Ditto.
* gcc.c-torture/compile/vector-align-1.c: Add max alignment if pdp11.
Index: testsuite/gcc.c-torture/compile/pr55921.c
===
--- testsuite/gcc.c-torture/compile/pr55921.c (revision 260780)
+++ testsuite/gcc.c-torture/compile/pr55921.c (revision 260781)
@@ -1,4 +1,5 @@
/* PR tree-optimization/55921 */
+/* { dg-skip-if "Not enough registers" { "pdp11-*-*" } } */
typedef union
{
Index: testsuite/gcc.c-torture/compile/vector-align-1.c
===
--- testsuite/gcc.c-torture/compile/vector-align-1.c(revision 260780)
+++ testsuite/gcc.c-torture/compile/vector-align-1.c(revision 260781)
@@ -2,7 +2,11 @@
/* If some target has a Max alignment less than 128, please create
a #ifdef around the alignment and add your alignment. */
+#ifdef __pdp11__
+#define alignment 2
+#else
#define alignment 128
+#endif
char x __attribute__((aligned(alignment),vector_size(2)));
Index: testsuite/gcc.c-torture/compile/20151204.c
===
--- testsuite/gcc.c-torture/compile/20151204.c (revision 260780)
+++ testsuite/gcc.c-torture/compile/20151204.c (revision 260781)
@@ -1,4 +1,4 @@
-/* { dg-skip-if "Array too big" { "avr-*-*" } } */
+/* { dg-skip-if "Array too big" { "avr-*-*" "pdp11-*-*" } } */
typedef __SIZE_TYPE__ size_t;
Index: testsuite/gcc.c-torture/compile/pr60655-1.c
===
--- testsuite/gcc.c-torture/compile/pr60655-1.c (revision 260780)
+++ testsuite/gcc.c-torture/compile/pr60655-1.c (revision 260781)
@@ -1,4 +1,4 @@
-/* { dg-options "-fdata-sections" { target { { ! { { hppa*-*-hpux* } && { !
lp64 } } } && { ! nvptx-*-* } } } } */
+/* { dg-options "-fdata-sections" { target { { ! { { hppa*-*-hpux* } && { !
lp64 } } } && { ! "nvptx-*-* pdp11-*-*" } } } } */
typedef unsigned char unit;
typedef unit *unitptr;
[PATCH, committed] fix ICE in test suite on pdp11
This cures an ICE caused by a wrong pattern that produces invalid register references. gcc/ChangeLog 2018-05-27 Paul Koning * config/pdp11/pdp11.md (truncsihi2): Remove. Index: config/pdp11/pdp11.md === --- config/pdp11/pdp11.md (revision 260806) +++ config/pdp11/pdp11.md (revision 260807) @@ -314,7 +314,6 @@ (match_operand:DI 1 "general_operand" "rN,g"))] "" "* return output_move_multiple (operands);" -;; what's the mose expensive code - say twice movsi = 16 [(set_attr "length" "16,32")]) (define_insn "movsi" @@ -322,8 +321,6 @@ (match_operand:SI 1 "general_operand" "rN,IJ,IJ,g"))] "" "* return output_move_multiple (operands);" -;; what's the most expensive code ? - I think 8! -;; we could split it up and make several sub-cases... [(set_attr "length" "4,6,8,16")]) (define_insn "mov" @@ -426,14 +423,6 @@ [(set_attr "length" "0,2,4")]) -(define_expand "truncsihi2" - [(set (match_operand:HI 0 "nonimmediate_operand" "=g") - (subreg:HI - (match_operand:SI 1 "general_operand" "or") - 0))] - "" - "") - ;;- zero extension instructions
[PATCH] add udivhi3, umodhi3 functions to libgcc
This patch adds two files which implement HImode unsigned divide
and mod for GCC on platforms where that is needed and not provided
in hardware. The code is lifted from the corresponding SImode
functions, with the obvious tweaks.
I'm not sure if I can commit this.
paul
2018-05-29 Paul Koning
* udivhi3.c: New file.
* udivmodhi4.c: New file.
* config/pdp11/t-pdp11 (LIB2ADD): Add the above files.
Index: config/pdp11/t-pdp11
===
--- config/pdp11/t-pdp11(revision 260806)
+++ config/pdp11/t-pdp11(working copy)
@@ -1,5 +1,7 @@
LIB2ADD = $(srcdir)/udivmod.c \
$(srcdir)/udivmodsi4.c \
+ $(srcdir)/udivhi3.c \
+ $(srcdir)/udivmodhi4.c \
$(srcdir)/memcmp.c \
$(srcdir)/memcpy.c \
$(srcdir)/memmove.c \
Index: udivhi3.c
===
--- udivhi3.c (nonexistent)
+++ udivhi3.c (working copy)
@@ -0,0 +1,37 @@
+/* Copyright (C) 2000-2018 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+short udivmodhi4 ();
+
+short
+__udivhi3 (short a, short b)
+{
+ return udivmodhi4 (a, b, 0);
+}
+
+short
+__umodhi3 (short a, short b)
+{
+ return udivmodhi4 (a, b, 1);
+}
+
Property changes on: udivhi3.c
___
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Index: udivmodhi4.c
===
--- udivmodhi4.c(nonexistent)
+++ udivmodhi4.c(working copy)
@@ -0,0 +1,47 @@
+/* Copyright (C) 2000-2018 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+unsigned short
+udivmodhi4(unsigned short num, unsigned short den, int modwanted)
+{
+ unsigned short bit = 1;
+ unsigned short res = 0;
+
+ while (den < num && bit && !(den & (1<<15)))
+{
+ den <<=1;
+ bit <<=1;
+}
+ while (bit)
+{
+ if (num >= den)
+ {
+ num -= den;
+ res |= bit;
+ }
+ bit >>=1;
+ den >>=1;
+}
+ if (modwanted) return num;
+ return res;
+}
Property changes on: udivmodhi4.c
___
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Re: [PATCH] add udivhi3, umodhi3 functions to libgcc
> On May 29, 2018, at 4:17 PM, Jakub Jelinek wrote:
>
> On Tue, May 29, 2018 at 03:01:20PM -0400, Paul Koning wrote:
>> +short udivmodhi4 ();
>
> We do want real prototypes, not K&R declarations.
Fixed. I had copied that from the SImode file.
>
>> Added: svn:eol-style
>> ## -0,0 +1 ##
>> +native
>
> Why?
My svn is set up to do that automatically, but I see GCC doesn't use this.
Fixed.
I noticed the types in the udiv and umod functions are signed, that's also that
way in udivmod.c. Should I change those to unsigned?
paul
Index: config/pdp11/t-pdp11
===
--- config/pdp11/t-pdp11(revision 260806)
+++ config/pdp11/t-pdp11(working copy)
@@ -1,5 +1,7 @@
LIB2ADD = $(srcdir)/udivmod.c \
$(srcdir)/udivmodsi4.c \
+ $(srcdir)/udivhi3.c \
+ $(srcdir)/udivmodhi4.c \
$(srcdir)/memcmp.c \
$(srcdir)/memcpy.c \
$(srcdir)/memmove.c \
Index: udivhi3.c
===
--- udivhi3.c (nonexistent)
+++ udivhi3.c (working copy)
@@ -0,0 +1,37 @@
+/* Copyright (C) 2000-2018 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+unsigned short udivmodhi4(unsigned short, unsigned short, int);
+
+short
+__udivhi3 (short a, short b)
+{
+ return udivmodhi4 (a, b, 0);
+}
+
+short
+__umodhi3 (short a, short b)
+{
+ return udivmodhi4 (a, b, 1);
+}
+
Index: udivmodhi4.c
===
--- udivmodhi4.c(nonexistent)
+++ udivmodhi4.c(working copy)
@@ -0,0 +1,47 @@
+/* Copyright (C) 2000-2018 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+unsigned short
+udivmodhi4(unsigned short num, unsigned short den, int modwanted)
+{
+ unsigned short bit = 1;
+ unsigned short res = 0;
+
+ while (den < num && bit && !(den & (1<<15)))
+{
+ den <<=1;
+ bit <<=1;
+}
+ while (bit)
+{
+ if (num >= den)
+ {
+ num -= den;
+ res |= bit;
+ }
+ bit >>=1;
+ den >>=1;
+}
+ if (modwanted) return num;
+ return res;
+}
Re: [PATCH] add udivhi3, umodhi3 functions to libgcc
> On Jun 1, 2018, at 5:04 PM, Joseph Myers wrote: > > On Tue, 29 May 2018, Paul Koning wrote: > >> +unsigned short udivmodhi4(unsigned short, unsigned short, int); > > libgcc should not have any such non-static functions in the user > namespace; they should all start with __. That too is a problem in the previous code. Ok, it sounds like the right answer is to withdraw this patch and create a new one that handles both this issue and cleans up the way udivsi is done. That would replace the existing udivmod.c and udivmodsi4.c files. paul
[PATCH] Avoid optimizing memory references with side effects in compare-elim.c
This fixes an ICE if post-reload compare elimination is done and the target
supports post_inc and similar modes, as pdp11 does. The ICE is caused by a
generated PARALLEL that has the side effect twice, which is not legal.
(Ideally it would generate a similar RTL suitable for a matching constraint
with the side effect omitted; I may try for that later on if that is still
supported by the constraint machinery.)
Tested against my in-progress CCmode pdp11 target. Ok to commit?
paul
gcc/ChangeLog:
2018-06-05 Paul Koning
* compare-elim.c (addr_side_effect_check): New function.
(addr_side_effect_p): Ditto.
(try_merge_compare): Don't merge compare if address contains a
side effect.
Index: compare-elim.c
===
--- compare-elim.c (revision 261207)
+++ compare-elim.c (working copy)
@@ -127,6 +127,23 @@
static vec all_compares;
+/* Callback function used by addr_side_effect_p. */
+static int
+addr_side_effect_check (rtx mem ATTRIBUTE_UNUSED, rtx op ATTRIBUTE_UNUSED,
+ rtx dest ATTRIBUTE_UNUSED, rtx src ATTRIBUTE_UNUSED,
+ rtx srcoff ATTRIBUTE_UNUSED, void *arg ATTRIBUTE_UNUSED)
+{
+ return 1;
+}
+
+/* Check if addr has side effects (contains autoinc or autodec
+ operations). */
+static int
+addr_side_effect_p (rtx addr)
+{
+ return for_each_inc_dec (addr, addr_side_effect_check, NULL);
+}
+
/* Look for a "conforming" comparison, as defined above. If valid, return
the rtx for the COMPARE itself. */
@@ -690,6 +707,13 @@
return false;
rtx src = SET_SRC (set);
+
+ /* If the source uses addressing modes with side effects, we can't
+ do the merge because we'd end up with a PARALLEL that has two
+ instances of that side effect in it. */
+ if (addr_side_effect_p (src))
+return false;
+
rtx flags = maybe_select_cc_mode (cmp, src, CONST0_RTX (GET_MODE (src)));
if (!flags)
{
@@ -809,6 +833,12 @@
else
return false;
+ /* If the source uses addressing modes with side effects, we can't
+ do the merge because we'd end up with a PARALLEL that has two
+ instances of that side effect in it. */
+ if (addr_side_effect_p (cmp_src))
+return false;
+
/* Determine if we ought to use a different CC_MODE here. */
flags = maybe_select_cc_mode (cmp, cmp_src, in_b);
if (flags == NULL)
[PATCH] Avoid optimizing memory references with side effects in compare-elim.c
(Resending -- forgot to include a test case.)
This fixes an ICE if post-reload compare elimination is done and the target
supports post_inc and similar modes, as pdp11 does. The ICE is caused by a
generated PARALLEL that has the side effect twice, which is not legal.
(Ideally it would generate a similar RTL suitable for a matching constraint
with the side effect omitted; I may try for that later on if that is still
supported by the constraint machinery.)
Tested against my in-progress CCmode pdp11 target. Ok to commit?
paul
gcc/ChangeLog:
2018-06-05 Paul Koning
* compare-elim.c (addr_side_effect_check): New function.
(addr_side_effect_p): Ditto.
(try_merge_compare): Don't merge compare if address contains a
side effect.
* gcc.c-torture/compile/20180605-1.c: New test case.
Index: compare-elim.c
===
--- compare-elim.c (revision 261207)
+++ compare-elim.c (working copy)
@@ -127,6 +127,23 @@
static vec all_compares;
+/* Callback function used by addr_side_effect_p. */
+static int
+addr_side_effect_check (rtx mem ATTRIBUTE_UNUSED, rtx op ATTRIBUTE_UNUSED,
+ rtx dest ATTRIBUTE_UNUSED, rtx src ATTRIBUTE_UNUSED,
+ rtx srcoff ATTRIBUTE_UNUSED, void *arg ATTRIBUTE_UNUSED)
+{
+ return 1;
+}
+
+/* Check if addr has side effects (contains autoinc or autodec
+ operations). */
+static int
+addr_side_effect_p (rtx addr)
+{
+ return for_each_inc_dec (addr, addr_side_effect_check, NULL);
+}
+
/* Look for a "conforming" comparison, as defined above. If valid, return
the rtx for the COMPARE itself. */
@@ -690,6 +707,13 @@
return false;
rtx src = SET_SRC (set);
+
+ /* If the source uses addressing modes with side effects, we can't
+ do the merge because we'd end up with a PARALLEL that has two
+ instances of that side effect in it. */
+ if (addr_side_effect_p (src))
+return false;
+
rtx flags = maybe_select_cc_mode (cmp, src, CONST0_RTX (GET_MODE (src)));
if (!flags)
{
@@ -809,6 +833,12 @@
else
return false;
+ /* If the source uses addressing modes with side effects, we can't
+ do the merge because we'd end up with a PARALLEL that has two
+ instances of that side effect in it. */
+ if (addr_side_effect_p (cmp_src))
+return false;
+
/* Determine if we ought to use a different CC_MODE here. */
flags = maybe_select_cc_mode (cmp, cmp_src, in_b);
if (flags == NULL)
Index: testsuite/gcc.c-torture/compile/20180605-1.c
===
--- testsuite/gcc.c-torture/compile/20180605-1.c(nonexistent)
+++ testsuite/gcc.c-torture/compile/20180605-1.c(working copy)
@@ -0,0 +1,9 @@
+void f (int *p, int n)
+{
+int j = 0, k;
+
+for (int i = 0; i < n; i++)
+if ((k = *p++) > 0)
+j += k;
+return j;
+}
Re: [PATCH] Avoid optimizing memory references with side effects in compare-elim.c
> On Jun 5, 2018, at 3:54 PM, Richard Sandiford
> wrote:
>
> Paul Koning writes:
>> This fixes an ICE if post-reload compare elimination is done and the
>> target supports post_inc and similar modes, as pdp11 does. The ICE is
>> caused by a generated PARALLEL that has the side effect twice, which
>> is not legal.
>>
>> (Ideally it would generate a similar RTL suitable for a matching
>> constraint with the side effect omitted; I may try for that later on
>> if that is still supported by the constraint machinery.)
>>
>> Tested against my in-progress CCmode pdp11 target. Ok to commit?
>
> Thanks for doing the conversion :-)
It has a ways to go still. And more changes to compare-elim may be needed,
since pdp11 is an oddball that has two separate CC registers (one for the CPU,
one for the FP coprocessor). Compare-elim doesn't currently handle that.
Credit to Eric Botcazou, whose documentation makes this a whole lot easier.
> ...
> It looks like the more general side_effects_p might be better
> in both cases. It's simpler than defining a custom function,
> and I'm not convinced the pass would handle other kinds of
> side-effect correctly either.
Thanks, I overlooked that function. Yes, it's a better choice. For example,
compare-eliminating a volatile memory reference seems incorrect (it's not clear
how likely it is to happen, but it makes sense to skip that case).
That simplifies the patch, which now looks like this. Ok for trunk?
paul
gcc/ChangeLog:
2018-06-05 Paul Koning
* compare-elim.c (try_merge_compare): Don't merge compare if
address contains a side effect.
* gcc.c-torture/compile/20180605-1.c: New test case.
Index: compare-elim.c
===
--- compare-elim.c (revision 261207)
+++ compare-elim.c (working copy)
@@ -690,6 +690,13 @@
return false;
rtx src = SET_SRC (set);
+
+ /* If the source uses addressing modes with side effects, we can't
+ do the merge because we'd end up with a PARALLEL that has two
+ instances of that side effect in it. */
+ if (side_effects_p (src))
+return false;
+
rtx flags = maybe_select_cc_mode (cmp, src, CONST0_RTX (GET_MODE (src)));
if (!flags)
{
@@ -809,6 +816,12 @@
else
return false;
+ /* If the source uses addressing modes with side effects, we can't
+ do the merge because we'd end up with a PARALLEL that has two
+ instances of that side effect in it. */
+ if (side_effects_p (cmp_src))
+return false;
+
/* Determine if we ought to use a different CC_MODE here. */
flags = maybe_select_cc_mode (cmp, cmp_src, in_b);
if (flags == NULL)
Index: testsuite/gcc.c-torture/compile/20180605-1.c
===
--- testsuite/gcc.c-torture/compile/20180605-1.c(nonexistent)
+++ testsuite/gcc.c-torture/compile/20180605-1.c(working copy)
@@ -0,0 +1,9 @@
+void f (int *p, int n)
+{
+int j = 0, k;
+
+for (int i = 0; i < n; i++)
+if ((k = *p++) > 0)
+j += k;
+return j;
+}
Re: [PATCH] Avoid optimizing memory references with side effects in compare-elim.c
> On Jun 6, 2018, at 11:53 AM, Eric Botcazou wrote:
>
>> That simplifies the patch, which now looks like this. Ok for trunk?
>>
>> paul
>>
>> gcc/ChangeLog:
>>
>> 2018-06-05 Paul Koning
>>
>> * compare-elim.c (try_merge_compare): Don't merge compare if
>> address contains a side effect.
>> * gcc.c-torture/compile/20180605-1.c: New test case.
>
> The patch is OK, but its ChangeLog is not. :-) It does not modify one
> function but two (try_merge_compare and try_eliminate_compare). I would
> suggest using "diff -up" to generate patches with more context.
Yes, sorry. I normally use the subversion internal diff which doesn't do this.
Here is a corrected version. Ok with this change?
paul
gcc/ChangeLog:
2018-06-06 Paul Koning
* compare-elim.c (try_merge_compare): Don't merge compare if
address contains a side effect.
(try_eliminate_compare): Likewise.
* gcc.c-torture/compile/20180605-1.c: New test case.
Index: compare-elim.c
===
--- compare-elim.c (revision 261207)
+++ compare-elim.c (working copy)
@@ -690,6 +690,13 @@ try_merge_compare (struct comparison *cm
return false;
rtx src = SET_SRC (set);
+
+ /* If the source uses addressing modes with side effects, we can't
+ do the merge because we'd end up with a PARALLEL that has two
+ instances of that side effect in it. */
+ if (side_effects_p (src))
+return false;
+
rtx flags = maybe_select_cc_mode (cmp, src, CONST0_RTX (GET_MODE (src)));
if (!flags)
{
@@ -809,6 +816,12 @@ try_eliminate_compare (struct comparison
else
return false;
+ /* If the source uses addressing modes with side effects, we can't
+ do the merge because we'd end up with a PARALLEL that has two
+ instances of that side effect in it. */
+ if (side_effects_p (cmp_src))
+return false;
+
/* Determine if we ought to use a different CC_MODE here. */
flags = maybe_select_cc_mode (cmp, cmp_src, in_b);
if (flags == NULL)
Index: testsuite/gcc.c-torture/compile/20180605-1.c
===
--- testsuite/gcc.c-torture/compile/20180605-1.c(nonexistent)
+++ testsuite/gcc.c-torture/compile/20180605-1.c(working copy)
@@ -0,0 +1,9 @@
+void f (int *p, int n)
+{
+int j = 0, k;
+
+for (int i = 0; i < n; i++)
+if ((k = *p++) > 0)
+j += k;
+return j;
+}
Re: [PATCH] Avoid optimizing memory references with side effects in compare-elim.c
> On Jun 7, 2018, at 11:37 AM, Eric Botcazou wrote: > >> Here is a corrected version. Ok with this change? >> >> paul >> >> gcc/ChangeLog: >> >> 2018-06-06 Paul Koning >> >> * compare-elim.c (try_merge_compare): Don't merge compare if >> address contains a side effect. >> (try_eliminate_compare): Likewise. >> * gcc.c-torture/compile/20180605-1.c: New test case. > > OK for mainline, thanks. Thanks. I just realized that the testsuite has its own ChangeLog. So committed with the change message split between those two. paul gcc/ChangeLog: 2018-06-07 Paul Koning * compare-elim.c (try_merge_compare): Don't merge compare if address contains a side effect. (try_eliminate_compare): Likewise. gcc/testsuite/ChangeLog: 2018-06-07 Paul Koning * gcc.c-torture/compile/20180605-1.c: New test.
Re: [PATCH] Come up with Deprecated option flag.
> On Jun 8, 2018, at 7:09 AM, Martin Liška wrote: > > Hi. > > First follow-up MPX removal patch comes up with Deprecated option flag. > That prints warning for options that have no effect: Should this be mentioned in the internals manual (section 8.2)? paul
[PATCH, committed] fix pdp11 target test failures
This patch fixes a number of test suite failures caused by data too large for
the address space or alignment larger than supported by this target.
paul
testsuite/ChangeLog:
2018-06-22 Paul Koning
* gcc.c-torture/execute/builtins/lib/chk.c: Use smaller alignment
if pdp11.
* gcc.c-torture/compile/20010518-2.c: Skip if pdp11 -mint32.
* gcc.c-torture/compile/20040101-1.c: Ditto.
* gcc.c-torture/compile/20050622-1.c: Ditto.
* gcc.c-torture/compile/20080625-1.c: Ditto.
* gcc.c-torture/compile/20090107-1.c: Ditto.
* gcc.c-torture/compile/920501-12.c: Ditto.
* gcc.c-torture/compile/920501-4.c: Ditto.
* gcc.c-torture/compile/961203-1.c: Ditto.
* gcc.c-torture/compile/limits-externdecl.c: Ditto.
* gcc.c-torture/compile/pr25310.c: Ditto.
Index: testsuite/gcc.c-torture/execute/builtins/lib/chk.c
===
--- testsuite/gcc.c-torture/execute/builtins/lib/chk.c (revision 261896)
+++ testsuite/gcc.c-torture/execute/builtins/lib/chk.c (revision 261897)
@@ -3,10 +3,18 @@
#include
#endif
+/* If some target has a Max alignment less than 16, please create
+ a #ifdef around the alignment and add your alignment. */
+#ifdef __pdp11__
+#define ALIGNMENT 2
+#else
+#define ALIGNMENT 16
+#endif
+
extern void abort (void);
extern int inside_main;
-void *chk_fail_buf[256] __attribute__((aligned (16)));
+void *chk_fail_buf[256] __attribute__((aligned (ALIGNMENT)));
volatile int chk_fail_allowed, chk_calls;
volatile int memcpy_disallowed, mempcpy_disallowed, memmove_disallowed;
volatile int memset_disallowed, strcpy_disallowed, stpcpy_disallowed;
Index: testsuite/gcc.c-torture/compile/920501-4.c
===
--- testsuite/gcc.c-torture/compile/920501-4.c (revision 261896)
+++ testsuite/gcc.c-torture/compile/920501-4.c (revision 261897)
@@ -1,5 +1,6 @@
/* { dg-do assemble } */
/* { dg-skip-if "ptxas times out" { nvptx-*-* } { "-O1" } { "" } } */
+/* { dg-skip-if "Array too big" { "pdp11-*-*" } { "-mint32" } } */
foo ()
{
Index: testsuite/gcc.c-torture/compile/20080625-1.c
===
--- testsuite/gcc.c-torture/compile/20080625-1.c(revision 261896)
+++ testsuite/gcc.c-torture/compile/20080625-1.c(revision 261897)
@@ -1,4 +1,5 @@
/* { dg-require-effective-target int32plus } */
+/* { dg-skip-if "Array too big" { "pdp11-*-*" } { "-mint32" } } */
struct peakbufStruct {
unsigned int lnum [5000];
Index: testsuite/gcc.c-torture/compile/20040101-1.c
===
--- testsuite/gcc.c-torture/compile/20040101-1.c(revision 261896)
+++ testsuite/gcc.c-torture/compile/20040101-1.c(revision 261897)
@@ -1,4 +1,4 @@
-/* { dg-skip-if "not enough registers" { pdp11-*-* } { "-O[12s]" } { "" } } */
+/* { dg-skip-if "not enough registers" { pdp11-*-* } } */
typedef unsigned short uint16_t;
typedef unsigned int uint32_t;
Index: testsuite/gcc.c-torture/compile/20090107-1.c
===
--- testsuite/gcc.c-torture/compile/20090107-1.c(revision 261896)
+++ testsuite/gcc.c-torture/compile/20090107-1.c(revision 261897)
@@ -1,3 +1,5 @@
+/* { dg-skip-if "Array too big" { "pdp11-*-*" } { "-mint32" } } */
+
/* Verify that we don't ICE by forming invalid addresses for unaligned
doubleword loads (originally for PPC64). */
Index: testsuite/gcc.c-torture/compile/20010518-2.c
===
--- testsuite/gcc.c-torture/compile/20010518-2.c(revision 261896)
+++ testsuite/gcc.c-torture/compile/20010518-2.c(revision 261897)
@@ -1,4 +1,5 @@
/* { dg-do compile } */
+/* { dg-skip-if "Array too big" { "pdp11-*-*" } { "-mint32" } } */
/* Large static storage. */
Index: testsuite/gcc.c-torture/compile/920501-12.c
===
--- testsuite/gcc.c-torture/compile/920501-12.c (revision 261896)
+++ testsuite/gcc.c-torture/compile/920501-12.c (revision 261897)
@@ -1,4 +1,5 @@
/* { dg-do assemble } */
+/* { dg-skip-if "Array too big" { "pdp11-*-*" } { "-mint32" } } */
x(x){return 3 + x;}
a(x){int y[994]; return 3 + x;}
Index: testsuite/gcc.c-torture/compile/limits-externdecl.c
===
--- testsuite/gcc.c-torture/compile/limits-externdecl.c (revision 261896)
+++ testsuite/gcc.c-torture/compile/limits-externdecl.c (revision 261897)
@@ -1,4 +1,5 @@
/* { dg-skip-if "ptxas runs out
[PATCH, committed] Convert pdp11 back end to CCmode
This change converts the pdp11 back end to use CCmode rather than cc0. It is
functional and the testsuite compile sections runs at least as well as before.
There is additional work left to be done; for example, the compare elimination
pass, which this target uses, does not know how to deal with a machine that has
two condition code registers.
Committed.
paul
ChangeLog:
2018-06-27 Paul Koning
* common/config/pdp11/pdp11-common.c (pdp11_handle_option): Handle
mutually exclusive options.
* config/pdp11/constraints.md (h): New constraint.
(O): Update definition to match shift code generation.
(D): New constraint.
* config/pdp11/pdp11-modes.def (CCNZ): Define mode.
(CCFP): Remove.
* config/pdp11/pdp11-protos.h (int_no_side_effect_operand): New
function.
(output_jump): Change arguments.
(pdp11_fixed_cc_regs): New function.
(pdp11_cc_mode): Ditto.
(pdp11_expand_shift): Ditto.
(pdp11_assemble_shift): Ditto.
(pdp11_small_shift): Ditto.
(pdp11_branch_cost): Remove.
* config/pdp11/pdp11.c (pdp11_assemble_integer): Remove comments
from output.
(pdp11_register_move_cost): Update for CC registers.
(pdp11_rtx_costs): Add case for LSHIFTRT.
(pdp11_output_jump): Add CCNZ mode conditional branches.
(notice_update_cc_on_set): Remove.
(pdp11_cc_mode): New function.
(simple_memory_operand): Correct pre/post decrement case.
(no_side_effect_operand): New function.
(pdp11_regno_reg_class): Add CC_REGS class.
(pdp11_fixed_cc_regs): New function.
(pdp11_small_shift): New function.
(pdp11_expand_shift): New function to expand shift insns.
(pdp11_assemble_shift): New function to output shifts.
(pdp11_branch_cost): Remove.
(pdp11_modes_tieable_p): Make QI/HI modes tieable.
* config/pdp11/pdp11.h (SIZE_TYPE): Ensure 16-bit type.
(WCHAR_TYPE): Ditto.
(PTRDIFF_TYPE): Ditto.
(ADJUST_INSN_LENGTH): New macro.
(FIXED_REGISTERS): Add CC registers.
(CALL_USED_REGISTERS): Ditto.
(reg_class): Ditto.
(REG_CLASS_NAMES): Ditto.
(REG_CLASS_CONTENTS): Ditto.
(SELECT_CC_MODE): Use new function.
(TARGET_FLAGS_REGNUM): New macro.
(TARGET_FIXED_CONDITION_CODE_REGS): Ditto.
(cc0_reg_rtx): Remove.
(CC_STATUS_MDEP): Remove.
(CC_STATUS_MDEFP_INIT): Remove.
(CC_IN_FPU): Remove.
(NOTICE_UPDATE_CC): Remove.
(REGISTER_NAMES): Add CC registers.
(BRANCH_COST): Change to constant 1.
* config/pdp11/pdp11.md: Rewrite for CCmode condition code
handling.
* config/pdp11/pdp11.opt (mbcopy): Remove.
(mbcopy-builtin): Remove.
(mbranch-cheap): Remove.
(mbranch-expensive): Remove.
* config/pdp11/predicates.md (expand_shift_operand): Update to
match shift code generation.
(ccnz_operator): New predicate.
* doc/invoke.texi (PDP-11 Options): Remove deleted options
-mbcopy, -mbcopy-builtin, -mbranch-cheap, -mbranch-expensive.
Remove non-existent option -mabshi, -mno-abshi. Document mutually
exclusive options.
* doc/md.texi (PDP-11): Document new D and h constraints. Update
description of O constraint.
Index: common/config/pdp11/pdp11-common.c
===
--- common/config/pdp11/pdp11-common.c (revision 262195)
+++ common/config/pdp11/pdp11-common.c (working copy)
@@ -39,9 +39,27 @@ pdp11_handle_option (struct gcc_options *opts,
switch (code)
{
case OPT_m10:
- opts->x_target_flags &= ~(MASK_40 | MASK_45);
+ opts->x_target_flags &= ~(MASK_40 | MASK_45 | MASK_FPU | MASK_AC0 |
MASK_SPLIT);
return true;
+case OPT_m40:
+ opts->x_target_flags &= ~(MASK_45 | MASK_FPU | MASK_AC0 | MASK_SPLIT);
+ return true;
+
+case OPT_mfpu:
+ opts->x_target_flags &= ~MASK_40;
+ opts->x_target_flags |= MASK_45;
+ return true;
+
+case OPT_msoft_float:
+ opts->x_target_flags &= ~MASK_AC0;
+ return true;
+
+case OPT_msplit:
+ opts->x_target_flags &= ~MASK_40;
+ opts->x_target_flags |= MASK_45;
+ return true;
+
default:
return true;
}
Index: config/pdp11/constraints.md
===
--- config/pdp11/constraints.md (revision 262195)
+++ config/pdp11/constraints.md (working copy)
@@ -18,11 +18,14 @@
;; along with GCC; see the file COPYING3. If not see
;; <http://www.gnu.org/licenses/>.
+(define_register_constraint "a" "LOAD_FPU_REGS"
+ "FPU register that can be directly loaded from memory")
+
(define_register_constraint "f" "F
[PATCH, committed] Fix insn length in pdp11 shift patterns
This patch corrects the setting of the "length" attributes in the pdp11 target
for certain shift cases.
Committed.
paul
ChangeLog:
2018-06-28 Paul Koning
* config/pdp11/pdp11-protos.h (pdp11_shift_length): New function.
* config/pdp11/pdp11.c (pdp11_shift_length): New function.
* config/pdp11/pdp11.h (ADJUST_INSN_LENGTH): Remove.
* config/pdp11/pdp11.md: Correct "length" attribute calculation
for shift insn patterns.
Index: config/pdp11/pdp11-protos.h
===
--- config/pdp11/pdp11-protos.h (revision 262198)
+++ config/pdp11/pdp11-protos.h (working copy)
@@ -41,6 +41,7 @@ extern machine_mode pdp11_cc_mode (enum rtx_code,
extern bool pdp11_expand_shift (rtx *, rtx (*) (rtx, rtx, rtx),
rtx (*) (rtx, rtx, rtx));
extern const char * pdp11_assemble_shift (rtx *, machine_mode, int);
+extern int pdp11_shift_length (rtx *, machine_mode, int, bool);
extern bool pdp11_small_shift (int);
#endif /* RTX_CODE */
Index: config/pdp11/pdp11.c
===
--- config/pdp11/pdp11.c(revision 262198)
+++ config/pdp11/pdp11.c(working copy)
@@ -2020,6 +2020,35 @@ pdp11_assemble_shift (rtx *operands, machine_mode
return "";
}
+/* Figure out the length of the instructions that will be produced for
+ the given operands by pdp11_assemble_shift above. */
+int
+pdp11_shift_length (rtx *operands, machine_mode m, int code, bool
simple_operand_p)
+{
+ int shift_size;
+
+ /* Shift by 1 is 2 bytes if simple operand, 4 bytes if 2-word addressing
mode. */
+ shift_size = simple_operand_p ? 2 : 4;
+
+ /* In SImode, two shifts are needed per data item. */
+ if (m == E_SImode)
+shift_size *= 2;
+
+ /* If shifting by a small constant, the loop is unrolled by the
+ shift count. Otherwise, account for the size of the decrement
+ and branch. */
+ if (CONSTANT_P (operands[2]) && pdp11_small_shift (INTVAL (operands[2])))
+shift_size *= INTVAL (operands[2]);
+ else
+shift_size += 4;
+
+ /* Logical right shift takes one more instruction (CLC). */
+ if (code == LSHIFTRT)
+shift_size += 2;
+
+ return shift_size;
+}
+
/* Worker function for TARGET_TRAMPOLINE_INIT.
trampoline - how should i do it in separate i+d ?
Index: config/pdp11/pdp11.h
===
--- config/pdp11/pdp11.h(revision 262198)
+++ config/pdp11/pdp11.h(working copy)
@@ -123,22 +123,6 @@ extern const struct real_format pdp11_d_format;
/* Define this if move instructions will actually fail to work
when given unaligned data. */
#define STRICT_ALIGNMENT 1
-
-/* Adjust the length of shifts by small constant amounts. The base
- value (in "length" on input) is the length of a shift by one, not
- including the CLC in logical shifts. */
-#define ADJUST_INSN_LENGTH(insn, length) \
- if ((GET_CODE (insn) == ASHIFT || \
- GET_CODE (insn) == ASHIFTRT || \
- GET_CODE (insn) == LSHIFTRT) && \
- GET_CODE (XEXP (insn, 2)) == CONST_INT && \
- pdp11_small_shift (XINT (insn, 2))) \
-{\
- if (GET_CODE (insn) == LSHIFTRT) \
- length = (length * XINT (insn, 2)) + 2; \
- else \
- length *= XINT (insn, 2); \
-}
/* Standard register usage. */
Index: config/pdp11/pdp11.md
===
--- config/pdp11/pdp11.md (revision 262198)
+++ config/pdp11/pdp11.md (working copy)
@@ -1297,11 +1297,6 @@
;; used to reduce the amount of very similar code.
;;
;; First the insns used for small constant shifts.
-;
-;; The "length" attribute values are modified by the ADJUST_INSN_LENGTH
-;; macro for the small constant shift case (first two alternatives).
-;; For those, the value coded in the length attribute is the cost of just
-;; the shift for a single shift.
(define_insn "_sc"
[(set (match_operand:QHSint 0 "nonimmediate_operand" "=rD,Q")
(SHF:QHSint (match_operand:QHSint 1 "general_operand" "0,0")
@@ -1308,7 +1303,9 @@
(match_operand:HI 2 "expand_shift_operand" "O,O")))]
""
"* return pdp11_assemble_shift (operands, , );"
- [(set_attr "length" "2,4")])
+ [(set (attr "length")
+ (symbol_ref "pdp11_shift_length (operands, ,
+ , which_alternative == 0)"))])
;; Next, shifts that are done as a loop on base (11/10 class) machines.
;; This applies to shift counts too large to unroll, or variable shift
@@ -1320,7 +1317,9 @@
(clobber (match_dup 2))]
""
"* return pdp11_assemble_shift (operands,
[PATCH, committed] Add -mgnu-asm to pdp11 target, change -mdec-asm
The pdp11 target has long had -mdec-asm which was documented to generate DEC compatible assembly language output but actually produces GNU assembler output. This patch adds -mgnu-asm to do what -mdec-asm used to do, and -mdec-asm now does produces output acceptable to DEC Macro-11. Committed. paul ChangeLog: 2018-07-01 Paul Koning * common/config/pdp11/pdp11-common.c (pdp11_handle_option): Handle -munit-asm, -mgnu-asm, -mdec-asm. * config/pdp11/pdp11-protos.h (pdp11_gen_int_label): New. (pdp11_output_labelref): New. (pdp11_output_def): New. (pdp11_output_addr_vec_elt): New. * config/pdp11/pdp11.c: Use tab between opcode and operands. Use %# and %@ format codes. (pdp11_option_override): New. (TARGET_ASM_FILE_START_FILE_DIRECTIVE): Define. (pdp11_output_ident): New. (pdp11_asm_named_section): New. (pdp11_asm_init_sections): New. (pdp11_file_start): New. (pdp11_file_end): New. (output_ascii): Use .ascii/.asciz for -mdec-asm. (pdp11_asm_print_operand): Update %# and %$ for -mdec-asm. Add %o, like %c but octal. (pdp11_option_override): New. * config/pdp11/pdp11.h (TEXT_SECTION_ASM_OP): Update for -mdec-asm. (DATA_SECTION_ASM_OP): Ditto. (READONLY_DATA_SECTION_ASM_OP): New. (IS_ASM_LOGICAL_LINE_SEPARATOR): New. (ASM_GENERATE_INTERNAL_LABEL): Use new function. (ASM_OUTPUT_LABELREF): Ditto. (ASM_OUTPUT_DEF): Ditto. (ASM_OUTPUT_EXTERNAL): New. (ASM_OUTPUT_SOURCE_FILENAME): New. (ASM_OUTPUT_ADDR_VEC_ELT): Use new function. (ASM_OUTPUT_SKIP): Update for -mdec-asm. * config/pdp11/pdp11.md: Use tab between opcode and operands. Use %# and %@ format codes. * config/pdp11/pdp11.opt (mgnu-asm): New. (mdec-asm): Conflicts with -mgnu-asm and -munix-asm. (munix-asm): Conflicts with -mdec-asm and -mgnu-asm. * doc/invoke.txt (PDP-11 Options): Add -mgnu-asm. Index: common/config/pdp11/pdp11-common.c === --- common/config/pdp11/pdp11-common.c (revision 262287) +++ common/config/pdp11/pdp11-common.c (working copy) @@ -59,7 +59,16 @@ pdp11_handle_option (struct gcc_options *opts, opts->x_target_flags &= ~MASK_40; opts->x_target_flags |= MASK_45; return true; - + +case OPT_munix_asm: +case OPT_mgnu_asm: + targetm_common.have_named_sections = false; + return true; + +case OPT_mdec_asm: + targetm_common.have_named_sections = true; + return true; + default: return true; } Index: config/pdp11/pdp11-protos.h === --- config/pdp11/pdp11-protos.h (revision 262287) +++ config/pdp11/pdp11-protos.h (working copy) @@ -51,3 +51,7 @@ extern void pdp11_asm_output_var (FILE *, const ch extern void pdp11_expand_prologue (void); extern void pdp11_expand_epilogue (void); extern poly_int64 pdp11_push_rounding (poly_int64); +extern void pdp11_gen_int_label (char *, const char *, int); +extern void pdp11_output_labelref (FILE *, const char *); +extern void pdp11_output_def (FILE *, const char *, const char *); +extern void pdp11_output_addr_vec_elt (FILE *, int); Index: config/pdp11/pdp11.c === --- config/pdp11/pdp11.c(revision 262287) +++ config/pdp11/pdp11.c(working copy) @@ -212,7 +212,7 @@ static bool pdp11_scalar_mode_supported_p (scalar_ #undef TARGET_PREFERRED_OUTPUT_RELOAD_CLASS #define TARGET_PREFERRED_OUTPUT_RELOAD_CLASS pdp11_preferred_output_reload_class -#undef TARGET_LRA_P +#undef TARGET_LRA_P #define TARGET_LRA_P hook_bool_void_false #undef TARGET_LEGITIMATE_ADDRESS_P @@ -221,9 +221,30 @@ static bool pdp11_scalar_mode_supported_p (scalar_ #undef TARGET_CONDITIONAL_REGISTER_USAGE #define TARGET_CONDITIONAL_REGISTER_USAGE pdp11_conditional_register_usage +#undef TARGET_OPTION_OVERRIDE +#define TARGET_OPTION_OVERRIDE pdp11_option_override + +#undef TARGET_ASM_FILE_START_FILE_DIRECTIVE +#define TARGET_ASM_FILE_START_FILE_DIRECTIVE true + +#undef TARGET_ASM_OUTPUT_IDENT +#define TARGET_ASM_OUTPUT_IDENT pdp11_output_ident + #undef TARGET_ASM_FUNCTION_SECTION #define TARGET_ASM_FUNCTION_SECTION pdp11_function_section +#undef TARGET_ASM_NAMED_SECTION +#defineTARGET_ASM_NAMED_SECTION pdp11_asm_named_section + +#undef TARGET_ASM_INIT_SECTIONS +#define TARGET_ASM_INIT_SECTIONS pdp11_asm_init_sections + +#undef TARGET_ASM_FILE_START +#define TARGET_ASM_FILE_START pdp11_file_start + +#undef TARGET_ASM_FILE_END +#define TARGET_ASM_FILE_END pdp11_file_end + #undef TARGET_PRINT_OPERAND #define TARGET_PRINT_OPERAND pdp11_asm_print_operand @@ -238,6 +259,7 @@ static bool pdp11_scalar_mode_supported_
Re: [PATCH 04/25] SPECIAL_REGNO_P
> On Sep 13, 2018, at 10:08 AM, Andrew Stubbs wrote: > > On 13/09/18 11:01, Andrew Stubbs wrote: >> The assert is caused because the def-use chains indicate that SCC conflicts >> with itself. I suppose the question is why is it doing that, but it's >> probably do do with that being a special register that gets used in split2 >> (particularly by the addptrdi3 pattern). Although, those patterns are >> careful to save SCC to one side and then restore it again after, so I'd have >> thought the DF analysis would work out? > > I think I may have a theory on this one now > > The addptrdi3 pattern must use two 32-bit adds with a carry in SCC, but > addptr patterns are not allowed to clobber SCC, so the splitter carefully > saves and restores the old value. If you don't have machine operations that add without messing with condition codes, wouldn't it make sense to omit the definition of the add-pointer patterns? GCC will build things out of normal (CC-clobbering) adds if there are no add-pointer operations, which may well be more efficient in most cases than explicitly saving/restoring a CC that may in fact not matter right at that spot. paul
Re: [PATCH 04/25] SPECIAL_REGNO_P
> On Sep 13, 2018, at 10:39 AM, Andrew Stubbs wrote: > > On 13/09/18 15:16, Paul Koning wrote: >> If you don't have machine operations that add without messing with >> condition codes, wouldn't it make sense to omit the definition of the >> add-pointer patterns? GCC will build things out of normal >> (CC-clobbering) adds if there are no add-pointer operations, which >> may well be more efficient in most cases than explicitly >> saving/restoring a CC that may in fact not matter right at that >> spot. > > I thought the whole point of addptr is that it *is* needed when add > clobbers CC? As in, LRA spills are malformed without this. > > Did something change? The internals manual still says "It only needs to > be defined if addm3 sets the condition code." It's ambiguous, because the last sentence of that paragraph says "addm3 is used if addptrm3 is not defined." I don't know of any change in this area. All I know is that pdp11 has adds that clobber CC and it doesn't define addptrm3, relying on that last sentence. I've tried LRA and for the most part it compiles successfully, I suppose I should verify the generated code based on the point you raised. If I really have to define addptr, I'm in trouble because save/restore CC is not easy on pdp11. paul
Re: [PATCH 04/25] SPECIAL_REGNO_P
> On Sep 13, 2018, at 10:58 AM, Andrew Stubbs wrote: > > On 13/09/18 15:49, Paul Koning wrote: >> It's ambiguous, because the last sentence of that paragraph says "addm3 is >> used if addptrm3 is not defined." > > I didn't read that as ambiguous; I read it as addm3 is assumed to work fine > when addptr is not defined. > >> I don't know of any change in this area. All I know is that pdp11 has adds >> that clobber CC and it doesn't define addptrm3, relying on that last >> sentence. I've tried LRA and for the most part it compiles successfully, I >> suppose I should verify the generated code based on the point you raised. >> If I really have to define addptr, I'm in trouble because save/restore CC >> is not easy on pdp11. > > The code was added because we had a number of testcases that failed at > runtime without it. > > Admittedly, that was in a GCC 7 code-base, and I can't reproduce the failure > with one of those test cases now (with addptr deleted), but possibly that's > just noise. Possibly relevant is that pdp11 is a "type 2" CC setting target, one where the machine description doesn't mention CC until after reload. So if reload (LRA) is generating adds, the CC effect of that is invisible anyway until later passes that deal with the resulting clobbers and elimination, or not, of compares. If that's what this is all about, some documentation clarification would help. Can someone confirm (or refute) my guess? paul
Re: [PATCH] middle-end/81035: Mention that noreturn suppresses tail call optimization
> On Sep 21, 2018, at 2:17 PM, Florian Weimer wrote:
>
> * Segher Boessenkool:
>
>> On Fri, Sep 21, 2018 at 12:59:27PM +0200, Florian Weimer wrote:
>>> 2018-09-21 Florian Weimer
>>>
>>> PR middle-end/81035
>>> * doc/extend.texi (Common Function Attributes): Mention that
>>> noreturn suppresses tail call optimization.
>>
>>> +In order to preserve backtraces, GCC will never turn calls to
>>> +@code{noreturn} functions into tail calls.
>>
>> Should we document this? Shouldn't we fix it, instead?
>
> Fix how? What is currently broken?
>
> For things like assertion failures, we do not want to replace the
> current stack frame with that of __assert_fail, I think.
I agree. Also, tailcalls are optimizations. Speed optimizing noreturn calls
is obviously not interesting. Calls to noreturn functions are short, and
turning them into a jump probably makes no difference in size, or if it does,
not enough to matter.
paul
[PATCH, pdp11] Enable LRA for pdp11
This patch enables LRA register allocator support for pdp11. For the moment,
it is invoked via a switch (-mlra) as is done by a few other targets. There
are some code quality issues and test suite rejects to be looked at, but it's
close enough for initial delivery.
Thanks to Segher for pointing out that LRA requires define_memory_constraint,
while the old allocator is happy when memory operands use define_constraint.
Committed.
paul
ChangeLog:
2018-10-03 Paul Koning
Enable LRA register allocator for PDP11.
* config/pdp11/constraints.md (Q): Use define_memory_constraints.
(R): Likewise.
(D): Likewise.
* config/pdp11/pdp11.c (pdp11_lra_p): New function.
* config/pdp11/pdp11.opt (-mlra): New option.
* doc/invoke.texi (PDP-11 Options): Document -mlra.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 264818)
+++ doc/invoke.texi (revision 264819)
@@ -1007,7 +1007,7 @@ Objective-C and Objective-C++ Dialects}.
@gccoptlist{-mfpu -msoft-float -mac0 -mno-ac0 -m40 -m45 -m10 @gol
-mint32 -mno-int16 -mint16 -mno-int32 @gol
-mfloat32 -mno-float64 -mfloat64 -mno-float32 @gol
--msplit -munix-asm -mdec-asm -mgnu-asm}
+-msplit -munix-asm -mdec-asm -mgnu-asm -mlra}
@emph{picoChip Options}
@gccoptlist{-mae=@var{ae_type} -mvliw-lookahead=@var{N} @gol
@@ -22721,6 +22721,11 @@ Use DEC assembler syntax.
@item -mgnu-asm
@opindex mgnu-asm
Use GNU assembler syntax. This is the default.
+
+@item -mlra
+@opindex mlra
+Use the new LRA register allocator. By default, the old ``reload''
+allocator is used.
@end table
@node picoChip Options
Index: config/pdp11/constraints.md
===
--- config/pdp11/constraints.md (revision 264818)
+++ config/pdp11/constraints.md (revision 264819)
@@ -70,19 +70,19 @@
(and (match_code "const_double")
(match_test "op == CONST0_RTX (GET_MODE (op))")))
-(define_constraint "Q"
+(define_memory_constraint "Q"
"Memory reference that requires an additional word after the opcode"
(and (match_code "mem")
(match_test "memory_address_p (GET_MODE (op), XEXP (op, 0))
&& !simple_memory_operand (op, GET_MODE (op))")))
-(define_constraint "R"
+(define_memory_constraint "R"
"Memory reference that is encoded within the opcode"
(and (match_code "mem")
(match_test "memory_address_p (GET_MODE (op), XEXP (op, 0))
&& simple_memory_operand (op, GET_MODE (op))")))
-(define_constraint "D"
+(define_memory_constraint "D"
"Memory reference that is encoded within the opcode, and not push or pop"
(and (match_code "mem")
(match_test "memory_address_p (GET_MODE (op), XEXP (op, 0))
Index: config/pdp11/pdp11.c
===
--- config/pdp11/pdp11.c(revision 264818)
+++ config/pdp11/pdp11.c(revision 264819)
@@ -230,7 +230,7 @@ static bool pdp11_scalar_mode_supported_p (scalar_
#define TARGET_PREFERRED_OUTPUT_RELOAD_CLASS
pdp11_preferred_output_reload_class
#undef TARGET_LRA_P
-#define TARGET_LRA_P hook_bool_void_false
+#define TARGET_LRA_P pdp11_lra_p
#undef TARGET_LEGITIMATE_ADDRESS_P
#define TARGET_LEGITIMATE_ADDRESS_P pdp11_legitimate_address_p
@@ -991,6 +991,12 @@ pdp11_assemble_integer (rtx x, unsigned int size,
}
+static bool
+pdp11_lra_p (void)
+{
+ return TARGET_LRA;
+}
+
/* Register to register moves are cheap if both are general
registers. */
static int
Index: config/pdp11/pdp11.opt
===
--- config/pdp11/pdp11.opt (revision 264818)
+++ config/pdp11/pdp11.opt (revision 264819)
@@ -73,3 +73,7 @@ Target has split I&D.
munix-asm
Target RejectNegative Report Mask(UNIX_ASM) Negative(mdec-asm)
Use UNIX assembler syntax.
+
+mlra
+Target Report Mask(LRA)
+Use LRA register allocator
[PATCH, pdp11] remove -mfloat32, -mfloat64
This patch removes switches that allow the size of "float" to be either the
usual 4, or 8 -- which is also the size of "double". That second choice
creates problems for Fortran and violates the Fortran standard. I don't see a
reason for having the option; it certainly is not a familiar thing to do on
this machine.
Committed.
paul
ChangeLog:
2018-10-05 Paul Koning
* config/pdp11/pdp11.h (FLOAT_TYPE_SIZE): Always 32.
* config/pdp11/pdp11.opt (mfloat32): Remove.
(mfloat64): Remove.
* doc/invoke.texi (pdp11 -mfloat32): Remove:
(pdp11 -mfloat64): Remove.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 264880)
+++ doc/invoke.texi (revision 264881)
@@ -1007,7 +1007,6 @@ Objective-C and Objective-C++ Dialects}.
@emph{PDP-11 Options}
@gccoptlist{-mfpu -msoft-float -mac0 -mno-ac0 -m40 -m45 -m10 @gol
-mint32 -mno-int16 -mint16 -mno-int32 @gol
--mfloat32 -mno-float64 -mfloat64 -mno-float32 @gol
-msplit -munix-asm -mdec-asm -mgnu-asm -mlra}
@emph{picoChip Options}
@@ -22722,18 +22721,6 @@ Use 16-bit @code{int}. This is the default.
@opindex mno-int16
Use 32-bit @code{int}.
-@item -mfloat64
-@itemx -mno-float32
-@opindex mfloat64
-@opindex mno-float32
-Use 64-bit @code{float}. This is the default.
-
-@item -mfloat32
-@itemx -mno-float64
-@opindex mfloat32
-@opindex mno-float64
-Use 32-bit @code{float}.
-
@item -msplit
@opindex msplit
Target has split instruction and data space. Implies -m45.
Index: config/pdp11/pdp11.opt
===
--- config/pdp11/pdp11.opt (revision 264880)
+++ config/pdp11/pdp11.opt (revision 264881)
@@ -42,14 +42,6 @@ mgnu-asm
Target RejectNegative Report Mask(GNU_ASM) Negative(munix-asm)
Use the GNU assembler syntax.
-mfloat32
-Target Report Mask(FLOAT32)
-Use 32 bit float.
-
-mfloat64
-Target Report InverseMask(FLOAT32, FLOAT64)
-Use 64 bit float.
-
mfpu
Target RejectNegative Report Mask(FPU)
Use hardware floating point.
Index: config/pdp11/pdp11.h
===
--- config/pdp11/pdp11.h(revision 264880)
+++ config/pdp11/pdp11.h(revision 264881)
@@ -59,12 +59,14 @@ along with GCC; see the file COPYING3. If not see
#define LONG_TYPE_SIZE 32
#define LONG_LONG_TYPE_SIZE64
-/* if we set FLOAT_TYPE_SIZE to 32, we could have the benefit
- of saving core for huge arrays - the definitions are
- already in md - but floats can never reside in
- an FPU register - we keep the FPU in double float mode
- all the time !! */
-#define FLOAT_TYPE_SIZE(TARGET_FLOAT32 ? 32 : 64)
+/* In earlier versions, FLOAT_TYPE_SIZE was selectable as 32 or 64,
+ but that conflicts with Fortran language rules. Since there is no
+ obvious reason why we should have that feature -- other targets
+ generally don't have float and double the same size -- I've removed
+ it. Note that it continues to be true (for now) that arithmetic is
+ always done with 64-bit values, i.e., the FPU is always in "double"
+ mode. */
+#define FLOAT_TYPE_SIZE32
#define DOUBLE_TYPE_SIZE 64
#define LONG_DOUBLE_TYPE_SIZE 64
@@ -200,12 +202,11 @@ extern const struct real_format pdp11_d_format;
MUL_REGS are used for odd numbered regs, to use in 16-bit multiplication
(even numbered do 32-bit multiply)
-LMUL_REGS long multiply registers (even numbered regs )
- (don't need them, all 32-bit regs are even numbered!)
GENERAL_REGS is all cpu
LOAD_FPU_REGS is the first four cpu regs, they are easier to load
NO_LOAD_FPU_REGS is ac4 and ac5, currently - difficult to load them
FPU_REGS is all fpu regs
+CC_REGS is the condition codes (CPU and FPU)
*/
enum reg_class
[PATCH, pdp11] Fix LRA failure
This patch fixes a failure handling block moves when the LRA register allocator
is used.
Committed.
paul
ChangeLog:
2018-10-08 Paul Koning
* config/pdp11/pdp11-protos.h (output_block_move): Remove.
(expand_block_move): New function.
* config/pdp11/pdp11.c (output_block_move): Remove.
(expand_block_move): New function.
* config/pdp11/pdp11.h (MOVE_RATIO): New definition.
* config/pdp11/pdp11.md (movmemhi): Use expand_block_move.
(*movmemhi1): Remove.
Index: config/pdp11/pdp11-protos.h
===
--- config/pdp11/pdp11-protos.h (revision 264929)
+++ config/pdp11/pdp11-protos.h (revision 264930)
@@ -26,7 +26,7 @@ extern int legitimate_const_double_p (rtx);
extern void notice_update_cc_on_set (rtx, rtx);
extern void output_addr_const_pdp11 (FILE *, rtx);
extern const char *output_move_multiple (rtx *);
-extern const char *output_block_move (rtx *);
+extern void expand_block_move (rtx *);
extern const char *output_jump (rtx *, int, int);
extern void print_operand_address (FILE *, rtx);
typedef enum { no_action, dec_before, inc_after } pdp11_action;
Index: config/pdp11/pdp11.md
===
--- config/pdp11/pdp11.md (revision 264929)
+++ config/pdp11/pdp11.md (revision 264930)
@@ -570,48 +570,20 @@
clrf\t%0"
[(set_attr "length" "2,2,4,4,2")])
-;; maybe fiddle a bit with move_ratio, then
-;; let constraints only accept a register ...
-
+;; Expand a block move. We turn this into a move loop.
(define_expand "movmemhi"
- [(parallel [(set (match_operand:BLK 0 "general_operand" "=g,g")
- (match_operand:BLK 1 "general_operand" "g,g"))
- (use (match_operand:HI 2 "general_operand" "n,mr"))
- (use (match_operand:HI 3 "immediate_operand" "i,i"))
- (clobber (match_scratch:HI 6 "=&r,X"))
- (clobber (match_dup 4))
- (clobber (match_dup 5))
- (clobber (match_dup 2))])]
+ [(match_operand:BLK 0 "general_operand" "=g")
+ (match_operand:BLK 1 "general_operand" "g")
+ (match_operand:HI 2 "immediate_operand" "i")
+ (match_operand:HI 3 "immediate_operand" "i")]
""
"
{
- operands[0]
-= replace_equiv_address (operands[0],
-copy_to_mode_reg (Pmode, XEXP (operands[0], 0)));
- operands[1]
-= replace_equiv_address (operands[1],
-copy_to_mode_reg (Pmode, XEXP (operands[1], 0)));
-
- operands[4] = XEXP (operands[0], 0);
- operands[5] = XEXP (operands[1], 0);
+ if (INTVAL (operands[2]) != 0)
+expand_block_move (operands);
+ DONE;
}")
-
-(define_insn "*movmemhi1"
- [(set (mem:BLK (match_operand:HI 0 "register_operand" "r,r"))
- (mem:BLK (match_operand:HI 1 "register_operand" "r,r")))
- (use (match_operand:HI 2 "general_operand" "n,r"))
- (use (match_operand:HI 3 "immediate_operand" "i,i"))
- (clobber (match_scratch:HI 4 "=&r,X"))
- (clobber (match_dup 0))
- (clobber (match_dup 1))
- (clobber (match_dup 2))]
- ""
- "* return output_block_move (operands);"
-;;; just a guess
- [(set_attr "length" "80")])
-
-
;;- truncation instructions
Index: config/pdp11/pdp11.c
===
--- config/pdp11/pdp11.c(revision 264929)
+++ config/pdp11/pdp11.c(revision 264930)
@@ -45,6 +45,7 @@ along with GCC; see the file COPYING3. If not see
#include "expr.h"
#include "builtins.h"
#include "dbxout.h"
+#include "explow.h"
#include "expmed.h"
/* This file should be included last. */
@@ -1513,173 +1514,48 @@ no_side_effect_operand(rtx op, machine_mode mode A
/*
- * output a block move:
+ * expand a block move:
*
* operands[0] ... to
* operands[1] ... from
* operands[2] ... length
* operands[3] ... alignment
- * operands[4] ... scratch register
*/
-
-const char *
-output_block_move(rtx *operands)
+void
+expand_block_move(rtx *operands)
{
-static int count = 0;
-char buf[200];
-int unroll;
-int lastbyte = 0;
-
-/* Move of zero bytes is a NOP. */
-if (operands[2] == const0_rtx)
- return "";
-
-/* Look for moves by small constant byte counts, those we'll
- expand to straight line code. */
-if (CONSTANT_P (operands[2]))
-{
- if (INTVAL (operands[2]) < 16
- && (!optimize_size || INTVAL (operands[2]) < 5)
- &am
[PATCH, pdp11] libgcc: remove -mfloat32
I missed a file that needed to be updated for the removal of -mfloat32. Committed. paul ChangeLog: 2018-10-08 Paul Koning * config/pdp11/t-pdp11: Remove -mfloat32 switch. Index: config/pdp11/t-pdp11 === --- config/pdp11/t-pdp11(revision 264938) +++ config/pdp11/t-pdp11(working copy) @@ -5,4 +5,4 @@ LIB2ADD = $(srcdir)/udivmod.c \ $(srcdir)/memmove.c \ $(srcdir)/memset.c -HOST_LIBGCC2_CFLAGS += -O2 -mfloat32 +HOST_LIBGCC2_CFLAGS += -O2
[PATCH, libgcc] Bug fix in udivmodhi4.c
This fixes a cut & paste oversight in udivmodhi4 (which is currently used only
by the pdp11 target) reported by Stefan Kanthak.
Committed as obvious.
paul
ChangeLog:
2018-12-05 Paul Koning
* udivmodhi4.c (__udivmodhi4): Fix loop end check.
Index: libgcc/udivmodhi4.c
===
--- libgcc/udivmodhi4.c (revision 266823)
+++ libgcc/udivmodhi4.c (working copy)
@@ -27,7 +27,7 @@ __udivmodhi4(unsigned short num, unsigned short de
unsigned short bit = 1;
unsigned short res = 0;
- while (den < num && bit && !(den & (1L<<31)))
+ while (den < num && bit && !(den & (1U<<15)))
{
den <<=1;
bit <<=1;
Re: [ping] Change static chain to r11 on aarch64
> On Dec 12, 2018, at 5:12 PM, Uecker, Martin > wrote: >> ... >> I've not seen such an alternative implementation (-fno-trampolines is >> ignored on all targets I tried), > > It was implemented for Ada. But here is a patch to also > activate it for C: > > https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00853.html > > With this patch one can use nested functions in C without > having an executable stack. That's interesting, because the internals manual (section 18.11, "Support for nested functions") clearly implies that it's already supported so long as the target defines the necessary macros / target hooks. Though admittedly the documentation is a bit muddled. I've been wanting to use this, since executable stacks really need to be avoided on pdp11. paul
Re: [PATCH v4][C][ADA] use function descriptors instead of trampolines in C
> On Dec 17, 2018, at 2:23 PM, Szabolcs Nagy wrote: > > On 17/12/2018 18:22, Uecker, Martin wrote: >>> >>> ... >> >> So a thread_local static variable for storing the static >> chain? > > something like that, but the more i think about it the > harder it seems: the call site of the nested function > may not be under control of the nested function writer, > in particular the nested function may be called on a > different thread, and extern library apis are unlikely > to provide guarantees about this, so in general if a > nested function escapes into an extern library then > this cannot be relied on, which limits my original > idea again to cases where there is no escape (which i > think is not that useful). I'm not sure I understand "escape" of a nested function pointer. Your description makes it sound like you're talking about a function being called by someone who has been given the pointer, from outside the scope of the function. That sounds like an illegal operation, exactly as it would be if you attempted to reference an automatic variable via a pointer from outside the scope of that variable. Did I misunderstand? paul
Re: [PATCH][ARM] Correctly set SLOW_BYTE_ACCESS
> On Sep 11, 2019, at 11:48 AM, Wilco Dijkstra wrote: > > Contrary to all documentation, SLOW_BYTE_ACCESS simply means accessing > bitfields by their declared type, which results in better codegeneration > on practically any target. So set it correctly to 1 on Arm. If the documentation is indeed wrong, could you submit a patch to cure that documentation defect? paul
Re: Deprecating cc0 (and consequently cc0 targets)
> On Sep 20, 2019, at 1:45 PM, Jeff Law wrote: > > On 9/20/19 11:22 AM, Richard Biener wrote: >> ... >> It seems to be that for the goal to keep a target alive a variant #2 >> conversion (according to the wiki) should be closely mirror CC0 >> behavior and thus should be easier to achieve and with less patterns >> touched than the 'good' variant. Can you acknowledge that and would >> such 'simple' conversions be OK? > Unless the target has condition code preserving arithmetic a variant #2 > conversion is the only way to go. Yes. > Now if someone did a variant #2 without the optimization bits (ie, > adding appropriate _set_flags pattern variants), I think that should be > considered explicitly OK even though the code quality would potentially > suffer. Essentially it's a choice between dropping the port or keeping > the port, but with slightly less optimization. THe latter seems like a > better choice to me. True. Then again, the added work of creating the pattern pairs is modest. With define_subst, much of the work can be automated. For pdp11, I found this to be the case; the hard part was to learn what is needed, and for that the Wiki ends up a big help. Also, pdp11 is harder than most because it has two CC registers (one for float ops, one for the rest). I don't know all the others, but for example VAX only has one, and I'm pretty sure the same applies to m68k. paul
[PATCH, doc] describe mode checking for doloop_end pattern
Since the code that uses the doloop_end pattern does not check the operand mode
as given in the pattern, the pattern itself may need to do this, and that was
not documented. This patch adds that information. It also updates the example
to reflect this.
Ok for trunk?
paul
ChangeLog:
2018-10-11 Paul Koning
* doc/md.texi (doloop_end): Document that the pattern code may
need to check operand mode.
Index: doc/md.texi
===
--- doc/md.texi (revision 265042)
+++ doc/md.texi (working copy)
@@ -7619,7 +7619,23 @@ simplified) from the PDP-11 target:
@smallexample
@group
-(define_insn "doloop_end"
+(define_expand "doloop_end"
+ [(parallel [(set (pc)
+ (if_then_else
+(ne (match_operand:HI 0 "nonimmediate_operand" "+r,!m")
+(const_int 1))
+(label_ref (match_operand 1 "" ""))
+(pc)))
+ (set (match_dup 0)
+ (plus:HI (match_dup 0)
+ (const_int -1)))])]
+ "TARGET_40_PLUS"
+ "@{
+if (GET_MODE (operands[0]) != HImode)
+ FAIL;
+ @}")
+
+(define_insn "doloop_end_nocc"
[(set (pc)
(if_then_else
(ne (match_operand:HI 0 "nonimmediate_operand" "+r,!m")
@@ -7628,17 +7644,28 @@ simplified) from the PDP-11 target:
(pc)))
(set (match_dup 0)
(plus:HI (match_dup 0)
- (const_int -1)))]
- ""
-
+ (const_int -1)))
+ (clobber (reg:CC CC_REGNUM))]
+ "TARGET_40_PLUS && reload_completed"
+ "*
@{
+rtx lb[1];
+
if (which_alternative == 0)
- return "sob %0,%l1";
+ return \"sob\t%0,%l1\";
+
+/* emulate sob */
+lb[0] = gen_label_rtx ();
+output_asm_insn (\"dec\t%0\", operands);
+output_asm_insn (\"beq\t%l0\", lb);
+output_asm_insn (\"jmp\t%l1\", operands);
+
+output_asm_label (lb[0]);
+fputs (\":\\n\", asm_out_file);
+
+return \"\";
+ @}")
-/* emulate sob */
-output_asm_insn ("dec %0", operands);
-return "bne %l1";
- @})
@end group
@end smallexample
@@ -7662,10 +7689,18 @@ will be non-negative.
Since the @code{doloop_end} insn is a jump insn that also has an output,
the reload pass does not handle the output operand. Therefore, the
constraint must allow for that operand to be in memory rather than a
-register. In the example shown above, that is handled by using a loop
-instruction sequence that can handle memory operands when the memory
-alternative appears.
+register. In the example shown above, that is handled (in the
+@code{doloop_end_nocc} pattern) by using a loop instruction sequence
+that can handle memory operands when the memory alternative appears.
+GCC does not check the mode of the loop register operand when generating
+the @code{doloop_end} pattern. If the pattern is only valid for some
+modes but not others, the pattern should be a @code{define_expand}
+pattern that checks the operand mode in the preparation code, and issues
+@code{FAIL} if an unsupported mode is found. The example above does
+this, since the machine instruction to be used only exists for
+@code{HImode}.
+
@end ifset
@ifset INTERNALS
@node Insn Canonicalizations
[PATCH, doc] describe mode checking for doloop_end pattern
Updated with an additional item I just debugged.
Since the code that uses the doloop_end pattern does not check the operand mode
as given in the pattern, the pattern itself may need to do this, and that was
not documented. In addition, if the doloop_end pattern is a define_expand,
there must be a define_insn (or define_insn_and_split) matching the generated
pattern. I had a define_split instead, and the result was an ICE in loop
optimization (loop2_done pass).
This patch adds that information. It also updates the example to reflect this.
Ok for trunk?
paul
ChangeLog:
2018-10-11 Paul Koning
* doc/md.texi (doloop_end): Document that the pattern code may
need to check operand mode.
Index: md.texi
===
--- md.texi (revision 265042)
+++ md.texi (working copy)
@@ -7619,7 +7619,23 @@ simplified) from the PDP-11 target:
@smallexample
@group
-(define_insn "doloop_end"
+(define_expand "doloop_end"
+ [(parallel [(set (pc)
+ (if_then_else
+(ne (match_operand:HI 0 "nonimmediate_operand" "+r,!m")
+(const_int 1))
+(label_ref (match_operand 1 "" ""))
+(pc)))
+ (set (match_dup 0)
+ (plus:HI (match_dup 0)
+ (const_int -1)))])]
+ ""
+ "@{
+if (GET_MODE (operands[0]) != HImode)
+ FAIL;
+ @}")
+
+(define_insn "doloop_end_insn"
[(set (pc)
(if_then_else
(ne (match_operand:HI 0 "nonimmediate_operand" "+r,!m")
@@ -7662,10 +7678,23 @@ will be non-negative.
Since the @code{doloop_end} insn is a jump insn that also has an output,
the reload pass does not handle the output operand. Therefore, the
constraint must allow for that operand to be in memory rather than a
-register. In the example shown above, that is handled by using a loop
-instruction sequence that can handle memory operands when the memory
-alternative appears.
+register. In the example shown above, that is handled (in the
+@code{doloop_end_nocc} pattern) by using a loop instruction sequence
+that can handle memory operands when the memory alternative appears.
+GCC does not check the mode of the loop register operand when generating
+the @code{doloop_end} pattern. If the pattern is only valid for some
+modes but not others, the pattern should be a @code{define_expand}
+pattern that checks the operand mode in the preparation code, and issues
+@code{FAIL} if an unsupported mode is found. The example above does
+this, since the machine instruction to be used only exists for
+@code{HImode}.
+
+If the @code{doloop_end} pattern is a @code{define_expand}, there must
+also be a @code{define_insn} or @code{define_insn_and_split} matching
+the generated pattern. Otherwise, the compiler will fail during loop
+optimization.
+
@end ifset
@ifset INTERNALS
@node Insn Canonicalizations
Re: [PATCH, doc] describe mode checking for doloop_end pattern
> On Oct 11, 2018, at 11:01 PM, Jeff Law wrote: > > On 10/11/18 3:09 PM, Paul Koning wrote: >> Updated with an additional item I just debugged. >> >> Since the code that uses the doloop_end pattern does not check the operand >> mode as given in the pattern, the pattern itself may need to do this, and >> that was not documented. In addition, if the doloop_end pattern is a >> define_expand, there must be a define_insn (or define_insn_and_split) >> matching the generated pattern. I had a define_split instead, and the >> result was an ICE in loop optimization (loop2_done pass). >> >> This patch adds that information. It also updates the example to reflect >> this. >> >> Ok for trunk? >> >> paul >> >> ChangeLog: >> >> 2018-10-11 Paul Koning >> >> * doc/md.texi (doloop_end): Document that the pattern code may >> need to check operand mode. > OK. > jeff Thanks. Committed. paul
[PATCH, pdp11] fix problem with doloop_end
For some newlib sources, pdp11 doloop_end was creating problems because it was
handed a QImode operand when it only wants HImode. This patch cures that, and
also adds addqi3 and subqi3 patterns to help with that case.
Committed.
paul
ChangeLog:
2018-10-12 Paul Koning
* config/pdp11/pdp11.md (doloop_end): New expander.
(doloop_end_insn): renamed from "doloop_end".
(addqi3): New pattern.
(subqi3): New pattern.
* config/pdp11/predicates.md (incdec_operand): New predicate.
Index: config/pdp11/predicates.md
===
--- config/pdp11/predicates.md (revision 265131)
+++ config/pdp11/predicates.md (revision 265132)
@@ -30,6 +30,14 @@
(and (match_code "const_int")
(match_test "(unsigned) INTVAL (op) < 4")))
+;; Accept integer arguments +1 and -1, for which add and sub can be
+;; done as inc or dec instructions. This matches the rule for the
+;; L and M constraints.
+(define_predicate "incdec_operand"
+ (and (match_code "const_int")
+ (ior (match_test "INTVAL (op) == -1")
+ (match_test "INTVAL (op) == 1"
+
;; Accept anything general_operand accepts, except that registers must
;; be FPU registers.
(define_predicate "float_operand"
Index: config/pdp11/pdp11.md
===
--- config/pdp11/pdp11.md (revision 265131)
+++ config/pdp11/pdp11.md (revision 265132)
@@ -251,9 +251,28 @@
;; sob instruction
;;
-;; Do a define_expand because some alternatives clobber CC.
+;; This expander has to check for mode match because the doloop pass
+;; in gcc that invokes it does not do so, i.e., it may attempt to apply
+;; this pattern even if the count operand is QI or SI mode.
+(define_expand "doloop_end"
+ [(parallel [(set (pc)
+ (if_then_else
+ (ne (match_operand:HI 0 "nonimmediate_operand" "+r,!m")
+ (const_int 1))
+ (label_ref (match_operand 1 "" ""))
+ (pc)))
+ (set (match_dup 0)
+ (plus:HI (match_dup 0)
+(const_int -1)))])]
+ "TARGET_40_PLUS"
+ "{
+if (GET_MODE (operands[0]) != HImode)
+ FAIL;
+ }")
+
+;; Do a define_split because some alternatives clobber CC.
;; Some don't, but it isn't all that interesting to cover that case.
-(define_insn_and_split "doloop_end"
+(define_insn_and_split "doloop_end_insn"
[(set (pc)
(if_then_else
(ne (match_operand:HI 0 "nonimmediate_operand" "+r,!m")
@@ -1067,6 +1086,35 @@
}"
[(set_attr "length" "2,4,4,6")])
+(define_insn_and_split "addqi3"
+ [(set (match_operand:QI 0 "nonimmediate_operand" "=rR,Q")
+ (plus:QI (match_operand:QI 1 "general_operand" "%0,0")
+(match_operand:QI 2 "incdec_operand" "LM,LM")))]
+ ""
+ "#"
+ "reload_completed"
+ [(parallel [(set (match_dup 0)
+ (plus:QI (match_dup 1) (match_dup 2)))
+ (clobber (reg:CC CC_REGNUM))])]
+ ""
+ [(set_attr "length" "2,4")])
+
+;; Inc/dec sets V if overflow from the operation
+(define_insn "*addqi3"
+ [(set (match_operand:QI 0 "nonimmediate_operand" "=rR,Q")
+ (plus:QI (match_operand:QI 1 "general_operand" "%0,0")
+(match_operand:QI 2 "incdec_operand" "LM,LM")))
+ (clobber (reg:CC CC_REGNUM))]
+ "reload_completed"
+ "*
+{
+ if (INTVAL(operands[2]) == 1)
+return \"incb\t%0\";
+ else
+return \"decb\t%0\";
+}"
+ [(set_attr "length" "2,4")])
+
;;- subtract instructions
;; we don't have to care for constant second
@@ -1226,6 +1274,35 @@
}"
[(set_attr "length" "2,4,4,6")])
+(define_insn_and_split "subqi3"
+ [(set (match_operand:QI 0 "nonimmediate_operand" "=rR,Q")
+ (plus:QI (match_operand:QI 1 "general_operand" "%0,0")
+(match_operand:QI 2 "incdec_operand" "LM,LM")))]
+ ""
+ "#"
+ "reload_completed"
+ [(parallel [(set (match_dup 0)
+ (plus:QI (match_dup 1) (match_dup 2)))
+ (clobber (reg:CC CC_REGNUM))])]
+ ""
+ [(set_attr "length" "2,4")])
+
+;; Inc/dec sets V if overflow from the operation
+(define_insn "*subqi3"
+ [(set (match_operand:QI 0 "nonimmediate_operand" "=rR,Q")
+ (plus:QI (match_operand:QI 1 "general_operand" "%0,0")
+(match_operand:QI 2 "incdec_operand" "LM,LM")))
+ (clobber (reg:CC CC_REGNUM))]
+ "reload_completed"
+ "*
+{
+ if (INTVAL(operands[2]) == -1)
+return \"incb\t%0\";
+ else
+return \"decb\t%0\";
+}"
+ [(set_attr "length" "2,4")])
+
- and instructions
;; Bit-and on the pdp (like on the VAX) is done with a clear-bits insn.
Re: [PATCH] add udivhi3, umodhi3 functions to libgcc
This is a revision of a patch I proposed a while back, to add udivhi3 and
umodhi3 functions to libgcc since some platforms (like pdp11) need it. The
code is adopted from that of udivsi3.
In earlier discussion it was pointed out that internal functions need to start
with __. The code I had copied does not do that, so I corrected mine and also
changed the existing code to conform to the rules.
Ok for trunk?
paul
ChangeLog:
2018-10-17 Paul Koning
* udivmodsi4.c (__udivmodsi4): Rename to conform to coding
standard.
* udivmod.c: Update references to __udivmodsi4.
* udivhi3.c: New file.
* udivmodhi4.c: New file.
* config/pdp11/t-pdp11 (LIB2ADD): Add the new files.
Index: config/pdp11/t-pdp11
===
--- config/pdp11/t-pdp11(revision 265151)
+++ config/pdp11/t-pdp11(working copy)
@@ -1,5 +1,7 @@
LIB2ADD = $(srcdir)/udivmod.c \
$(srcdir)/udivmodsi4.c \
+ $(srcdir)/udivhi3.c \
+ $(srcdir)/udivmodhi4.c \
$(srcdir)/memcmp.c \
$(srcdir)/memcpy.c \
$(srcdir)/memmove.c \
Index: udivhi3.c
===
--- udivhi3.c (nonexistent)
+++ udivhi3.c (working copy)
@@ -0,0 +1,38 @@
+/* Copyright (C) 2000-2018 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+extern unsigned short __udivmodhi4(unsigned short num, unsigned short den,
+ int
modwanted);
+
+unsigned short
+__udivhi3 (unsigned short a, unsigned short b)
+{
+ return __udivmodhi4 (a, b, 0);
+}
+
+unsigned short
+__umodhi3 (unsigned short a, unsigned short b)
+{
+ return __udivmodhi4 (a, b, 1);
+}
+
Index: udivmod.c
===
--- udivmod.c (revision 265151)
+++ udivmod.c (working copy)
@@ -21,17 +21,18 @@ a copy of the GCC Runtime Library Exception along
see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
<http://www.gnu.org/licenses/>. */
-long udivmodsi4 ();
+extern unsigned long __udivmodsi4(unsigned long num, unsigned long den,
+ int
modwanted);
long
__udivsi3 (long a, long b)
{
- return udivmodsi4 (a, b, 0);
+ return __udivmodsi4 (a, b, 0);
}
long
__umodsi3 (long a, long b)
{
- return udivmodsi4 (a, b, 1);
+ return __udivmodsi4 (a, b, 1);
}
Index: udivmodhi4.c
===
--- udivmodhi4.c(nonexistent)
+++ udivmodhi4.c(working copy)
@@ -0,0 +1,47 @@
+/* Copyright (C) 2000-2018 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+unsigned short
+__udivmodhi4(unsigned short num, unsigned short den, int modwanted)
+{
+ unsigned short bit = 1;
+ unsigned short res = 0;
+
+ while (den < num && bit && !(den & (1L<<31)))
+{
+ den <<=1;
+ bit <<=1;
+}
+ while (bit)
+{
+ if (num >= den)
+ {
+ num -= den;
+ res |= bit;
+ }
+
Re: [PATCH] add udivhi3, umodhi3 functions to libgcc
> On Oct 18, 2018, at 1:18 PM, Jeff Law wrote:
>
> On 10/17/18 5:48 PM, Paul Koning wrote:
>> This is a revision of a patch I proposed a while back, to add udivhi3 and
>> umodhi3 functions to libgcc since some platforms (like pdp11) need it. The
>> code is adopted from that of udivsi3.
>>
>> In earlier discussion it was pointed out that internal functions need to
>> start with __. The code I had copied does not do that, so I corrected mine
>> and also changed the existing code to conform to the rules.
>>
>> Ok for trunk?
>>
>> paul
>>
>> ChangeLog:
>>
>> 2018-10-17 Paul Koning
>>
>> * udivmodsi4.c (__udivmodsi4): Rename to conform to coding
>> standard.
>> * udivmod.c: Update references to __udivmodsi4.
>> * udivhi3.c: New file.
>> * udivmodhi4.c: New file.
>> * config/pdp11/t-pdp11 (LIB2ADD): Add the new files.
> I think you need to fix divmod.c as well since it calls udivmodsi4. OK
> with that fixed.
>
> Jeff
Thanks. Committed as shown below.
paul
ChangeLog:
2018-10-18 Paul Koning
* udivmodsi4.c (__udivmodsi4): Rename to conform to coding
standard.
* divmod.c: Update references to __udivmodsi4.
* udivmod.c: Ditto.
* udivhi3.c: New file.
* udivmodhi4.c: New file.
* config/pdp11/t-pdp11 (LIB2ADD): Add the new files.
Index: config/pdp11/t-pdp11
===
--- config/pdp11/t-pdp11(revision 265276)
+++ config/pdp11/t-pdp11(working copy)
@@ -1,5 +1,7 @@
LIB2ADD = $(srcdir)/udivmod.c \
$(srcdir)/udivmodsi4.c \
+ $(srcdir)/udivhi3.c \
+ $(srcdir)/udivmodhi4.c \
$(srcdir)/memcmp.c \
$(srcdir)/memcpy.c \
$(srcdir)/memmove.c \
Index: divmod.c
===
--- divmod.c(revision 265276)
+++ divmod.c(working copy)
@@ -21,7 +21,8 @@ a copy of the GCC Runtime Library Exception along
see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
<http://www.gnu.org/licenses/>. */
-long udivmodsi4 ();
+extern unsigned long __udivmodsi4(unsigned long num, unsigned long den,
+ int
modwanted);
long
__divsi3 (long a, long b)
@@ -41,7 +42,7 @@ __divsi3 (long a, long b)
neg = !neg;
}
- res = udivmodsi4 (a, b, 0);
+ res = __udivmodsi4 (a, b, 0);
if (neg)
res = -res;
@@ -64,7 +65,7 @@ __modsi3 (long a, long b)
if (b < 0)
b = -b;
- res = udivmodsi4 (a, b, 1);
+ res = __udivmodsi4 (a, b, 1);
if (neg)
res = -res;
Index: udivhi3.c
===
--- udivhi3.c (nonexistent)
+++ udivhi3.c (working copy)
@@ -0,0 +1,38 @@
+/* Copyright (C) 2000-2018 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC 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.
+
+GCC 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.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+extern unsigned short __udivmodhi4(unsigned short num, unsigned short den,
+ int
modwanted);
+
+unsigned short
+__udivhi3 (unsigned short a, unsigned short b)
+{
+ return __udivmodhi4 (a, b, 0);
+}
+
+unsigned short
+__umodhi3 (unsigned short a, unsigned short b)
+{
+ return __udivmodhi4 (a, b, 1);
+}
+
Index: udivmod.c
===
--- udivmod.c (revision 265276)
+++ udivmod.c (working copy)
@@ -21,17 +21,18 @@ a copy of the GCC Runtime Library Exception along
see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
<http://www.gnu.org/licenses/>. */
-long udivmodsi4 ();
+extern unsigned long __udivmodsi4(unsigned long num, unsigned long den,
+ int
modwanted);
long
__udivsi3 (long a, long b)
{
- return udivmodsi4 (a,
[PATCH] correct max alignment check in symtab.c
GCC was hitting an ICE on some pdp11 tests due to a typo in a max alignment
check.
Committed as obvious.
paul
ChangeLog:
2018-10-22 Paul Koning
* symtab.c (symtab_node::increase_alignment): Correct max
alignment check.
Index: symtab.c
===
--- symtab.c(revision 265403)
+++ symtab.c(working copy)
@@ -2205,7 +2205,7 @@ increase_alignment_1 (symtab_node *n, void *v)
void
symtab_node::increase_alignment (unsigned int align)
{
- gcc_assert (can_increase_alignment_p () && align < MAX_OFILE_ALIGNMENT);
+ gcc_assert (can_increase_alignment_p () && align <= MAX_OFILE_ALIGNMENT);
ultimate_alias_target()->call_for_symbol_and_aliases (increase_alignment_1,
(void *)(size_t) align,
true);
[PATCH, testsuite]: check for weak support
I ran into a failures due to no weak symbol support in my target. This patch
cures that. Is it right? The test case uses "weakref" so I' not 100% sure
that checking for "weak" support is correct. If not, I can put in a skip-if
check for the target (pdp11) instead.
paul
ChangeLog:
2018-10-25 Paul Koning
* gcc.dg/tree-ssa/attr-alias.c: Skip if no weak support.
Index: testsuite/gcc.dg/tree-ssa/attr-alias.c
===
--- testsuite/gcc.dg/tree-ssa/attr-alias.c (revision 265404)
+++ testsuite/gcc.dg/tree-ssa/attr-alias.c (working copy)
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-require-alias "" } */
+/* { dg-require-weak "" } */
/* { dg-options "-O2 -fdump-tree-optimized -std=gnu89" } */
void abort (void);
__attribute__ ((weak))
[PATCH, doc] Fix CONST_WIDE_INT_ELT
The description of CONST_WIDE_INT_ELT gave the macro's name as
CONST_WIDE_INT_NUNITS instead.
Committed as obvious.
paul
ChangeLog:
2018-10-29 Paul Koning
* doc/rtl.texi (CONST_WIDE_INT_ELT): Give correct macro name.
Index: doc/rtl.texi
===
--- doc/rtl.texi(revision 265601)
+++ doc/rtl.texi(working copy)
@@ -1732,7 +1732,7 @@ Note that this generally is smaller than the numbe
@code{HOST_WIDE_INT}s implied by the mode size.
@findex CONST_WIDE_INT_ELT
-@item CONST_WIDE_INT_NUNITS (@var{code},@var{i})
+@item CONST_WIDE_INT_ELT (@var{code},@var{i})
Returns the @code{i}th element of the array. Element 0 is contains
the low order bits of the constant.
Ping: [PATCH, testsuite]: check for weak support
Ping. Ok to commit?
paul
> On Oct 25, 2018, at 2:57 PM, Paul Koning wrote:
>
> I ran into a failures due to no weak symbol support in my target. This patch
> cures that. Is it right? The test case uses "weakref" so I' not 100% sure
> that checking for "weak" support is correct. If not, I can put in a skip-if
> check for the target (pdp11) instead.
>
> paul
>
> ChangeLog:
>
> 2018-10-25 Paul Koning
>
> * gcc.dg/tree-ssa/attr-alias.c: Skip if no weak support.
>
> Index: testsuite/gcc.dg/tree-ssa/attr-alias.c
> ===
> --- testsuite/gcc.dg/tree-ssa/attr-alias.c(revision 265404)
> +++ testsuite/gcc.dg/tree-ssa/attr-alias.c(working copy)
> @@ -1,5 +1,6 @@
> /* { dg-do compile } */
> /* { dg-require-alias "" } */
> +/* { dg-require-weak "" } */
> /* { dg-options "-O2 -fdump-tree-optimized -std=gnu89" } */
> void abort (void);
> __attribute__ ((weak))
>
Re: Ping: [PATCH, testsuite]: check for weak support
> On Oct 30, 2018, at 10:17 AM, Jeff Law wrote: > > On 10/30/18 6:55 AM, Paul Koning wrote: >> Ping. Ok to commit? >> >> paul >> >>> On Oct 25, 2018, at 2:57 PM, Paul Koning wrote: >>> >>> I ran into a failures due to no weak symbol support in my target. This >>> patch cures that. Is it right? The test case uses "weakref" so I' not >>> 100% sure that checking for "weak" support is correct. If not, I can put >>> in a skip-if check for the target (pdp11) instead. >>> >>> paul >>> >>> ChangeLog: >>> >>> 2018-10-25 Paul Koning >>> >>> * gcc.dg/tree-ssa/attr-alias.c: Skip if no weak support. > OK. This would fall under the obvious rule IMHO. There's a "weak" > attribute so clearly the test needs to require weak support :-) > > jeff Thanks. Committed. paul
[PATCH] libgcc build fix for pdp11
This fixes some test suite failures due to a missing arithmetic support routine. Committed. paul ChangeLog: 2018-11-01 Paul Koning * config/pdp11/t-pdp11 (LIB2ADD): Add divmod.c. (HOST_LIBGCC2_CFLAGS): Change to optimize for size. Index: config/pdp11/t-pdp11 === --- config/pdp11/t-pdp11(revision 265725) +++ config/pdp11/t-pdp11(working copy) @@ -1,4 +1,5 @@ -LIB2ADD = $(srcdir)/udivmod.c \ +LIB2ADD = $(srcdir)/divmod.c \ + $(srcdir)/udivmod.c \ $(srcdir)/udivmodsi4.c \ $(srcdir)/udivhi3.c \ $(srcdir)/udivmodhi4.c \ @@ -7,4 +8,5 @@ $(srcdir)/memmove.c \ $(srcdir)/memset.c -HOST_LIBGCC2_CFLAGS += -O2 +# Small machine, optimize for size +HOST_LIBGCC2_CFLAGS += -Os
[PATCH, testsuite] test case fixes for pdp11
This patch fixes a number of test case failures on pdp11. Some are too large
for the address space, some have dependencies on the float format that don't
match the DEC format, some add pdp11 to the targets that expect particular
compiler messages.
Committed.
paul
ChangeLog:
2018-11-01 Paul Koning
* gcc.c-torture/execute/20010904-1.c: Align 2 if pdp11.
* gcc.c-torture/execute/20010904-2.c: Ditto.
* c-c++-common/builtin-arith-overflow-2.c: Skip if pdp11.
* gcc.dg/Walloc-size-larger-than-4.c: Ditto.
* gcc.dg/Walloc-size-larger-than-5.c: Ditto.
* gcc.dg/Walloc-size-larger-than-6.c: Ditto.
* gcc.dg/Walloc-size-larger-than-7.c: Ditto.
* gcc.dg/Walloca-14.c: Ditto.
* gcc.dg/Wlarger-than3.c: Ditto.
* gcc.dg/compat/pr83487-1_y.c: Ditto.
* gcc.dg/compat/struct-by-value-2_x.c: Ditto.
* gcc.dg/compat/struct-by-value-3_x.c: Ditto.
* gcc.dg/compat/struct-by-value-4_x.c: Ditto.
* gcc.dg/compat/struct-by-value-5b_x.c: Ditto.
* gcc.dg/compat/struct-by-value-6b_x.c: Ditto.
* gcc.dg/compat/struct-by-value-7b_x.c: Ditto.
* gcc.dg/compat/struct-by-value-8_x.c: Ditto.
* gcc.dg/compat/struct-by-value-9_x.c: Ditto.
* gcc.dg/compat/struct-by-value-10_x.c: Ditto.
* gcc.dg/compat/struct-by-value-11_x.c: Ditto.
* gcc.dg/compat/struct-by-value-12_x.c: Ditto.
* gcc.dg/compat/struct-by-value-13_x.c: Ditto.
* gcc.dg/compat/struct-by-value-14_x.c: Ditto.
* gcc.dg/compat/struct-by-value-15_x.c: Ditto.
* gcc.dg/compat/struct-by-value-16_x.c: Ditto.
* gcc.dg/compat/struct-by-value-17_x.c: Ditto.
* gcc.dg/compat/struct-by-value-18_x.c: Ditto.
* gcc.dg/compat/struct-by-value-22_x.c: Ditto.
* gcc.dg/compat/struct-return-2_x.c: Ditto.
* gcc.dg/falign-labels-1.c: Ditto.
* gcc.dg/long_branch.c: Ditto.
* gcc.dg/nextafter-1.c: Ditto.
* gcc.dg/pr35045.c: Ditto.
* gcc.dg/pr48616.c: Ditto.
* gcc.dg/pr84100.c: Ditto.
* gcc.dg/tree-ssa/builtin-sprintf-9.c: Ditto.
* gcc.dg/tree-ssa/builtin-sprintf-warn-10.c: Ditto.
* gcc.dg/tree-ssa/builtin-sprintf.c: Ditto.
* gcc.dg/Wattributes-10.c: Expect error if pdp11.
* gcc.dg/attr-alloc_size-11.c: Don't XFAIL if pdp11.
* gcc.dg/builtin-inf-1.c: Add pdp11 to warnings about INF.
* gcc.dg/builtins-1.c: Ditto.
Index: gcc.c-torture/execute/20010904-1.c
===
--- gcc.c-torture/execute/20010904-1.c (revision 265727)
+++ gcc.c-torture/execute/20010904-1.c (working copy)
@@ -1,4 +1,12 @@
-typedef struct x { int a; int b; } __attribute__((aligned(32))) X;
+/* If some target has a Max alignment less than 32, please create
+ a #ifdef around the alignment and add your alignment. */
+#ifdef __pdp11__
+#define alignment 2
+#else
+#define alignment 32
+#endif
+
+typedef struct x { int a; int b; } __attribute__((aligned(alignment))) X;
typedef struct y { X x[32]; int c; } Y;
Y y[2];
Index: gcc.c-torture/execute/20010904-2.c
===
--- gcc.c-torture/execute/20010904-2.c (revision 265727)
+++ gcc.c-torture/execute/20010904-2.c (working copy)
@@ -1,4 +1,12 @@
-typedef struct x { int a; int b; } __attribute__((aligned(32))) X;
+/* If some target has a Max alignment less than 32, please create
+ a #ifdef around the alignment and add your alignment. */
+#ifdef __pdp11__
+#define alignment 2
+#else
+#define alignment 32
+#endif
+
+typedef struct x { int a; int b; } __attribute__((aligned(aligned))) X;
typedef struct y { X x; X y[31]; int c; } Y;
Y y[2];
Index: c-c++-common/builtin-arith-overflow-2.c
===
--- c-c++-common/builtin-arith-overflow-2.c (revision 265727)
+++ c-c++-common/builtin-arith-overflow-2.c (working copy)
@@ -1,7 +1,7 @@
/* PR c/68120 - can't easily deal with integer overflow at compile time */
/* { dg-do run } */
/* { dg-additional-options "-Wno-long-long" } */
-/* { dg-skip-if "Program too big" { "avr-*-*" } } */
+/* { dg-skip-if "Program too big" { "avr-*-* pdp11*-*-*" } } */
#define SCHAR_MAX__SCHAR_MAX__
#define SHRT_MAX __SHRT_MAX__
Index: gcc.dg/Walloc-size-larger-than-4.c
===
--- gcc.dg/Walloc-size-larger-than-4.c (revision 265727)
+++ gcc.dg/Walloc-size-larger-than-4.c (working copy)
@@ -1,5 +1,6 @@
/* PR middle-end/82063 - issues with arguments enabled by -Wall
{ dg-do compile }
+ { dg-skip-if "small address space" { "pdp11-*-*" } }
{ dg-options "-O -Walloc-size-larger-than=1MiB -ftrack-macro-expansion=0" }
*/
void sink (void*);
Index: gcc.dg/Walloc-size-larger
[PATCH, testsuite] ignore some "conflicting types for built-in" messages
A number of test cases contain declarations like:
void *memcpy();
which currently are silently accepted on most platforms but not on all; pdp11
(and possibly some others) generate a "conflicting types for built-in function"
warning.
It was suggested to prune those messages because the test cases where these
occur are not looking for the message but are testing some other issue, so the
message is not relevant. The attached patch adds dg-prune-output directives to
do so.
Ok for trunk?
paul
ChangeLog:
2018-11-01 Paul Koning
* gcc.dg/Walloca-16.c: Ignore conflicting types for built-in
warnings.
* gcc.dg/Wrestrict-4.c: Ditto.
* gcc.dg/Wrestrict-5.c: Ditto.
* gcc.dg/pr83463.c: Ditto.
* gcc.dg/torture/pr55890-2.c: Ditto.
* gcc.dg/torture/pr55890-3.c: Ditto.
* gcc.dg/torture/pr71816.c: Ditto.
Index: gcc.dg/Walloca-16.c
===
--- gcc.dg/Walloca-16.c (revision 265727)
+++ gcc.dg/Walloca-16.c (working copy)
@@ -1,5 +1,6 @@
/* PR tree-optimization/84224 */
/* { dg-do compile } */
+/* { dg-prune-output "conflicting types for built-in" } */
/* { dg-options "-O0 -Walloca" } */
void *alloca ();
Index: gcc.dg/Wrestrict-4.c
===
--- gcc.dg/Wrestrict-4.c(revision 265727)
+++ gcc.dg/Wrestrict-4.c(working copy)
@@ -3,6 +3,7 @@
Test to verify that invalid calls to built-in functions declared
without a prototype don't cause an ICE.
{ dg-do compile }
+ { dg-prune-output "conflicting types for built-in" }
{ dg-options "-O2 -Warray-bounds -Wrestrict" } */
void* memcpy ();
Index: gcc.dg/Wrestrict-5.c
===
--- gcc.dg/Wrestrict-5.c(revision 265727)
+++ gcc.dg/Wrestrict-5.c(working copy)
@@ -2,6 +2,7 @@
functions declared with no prototype are checked for overlap, and that
invalid calls are ignored.
{ dg-do compile }
+ { dg-prune-output "conflicting types for built-in" }
{ dg-options "-O2 -Wrestrict" } */
typedef __SIZE_TYPE__ size_t;
Index: gcc.dg/pr83463.c
===
--- gcc.dg/pr83463.c(revision 265727)
+++ gcc.dg/pr83463.c(working copy)
@@ -1,5 +1,6 @@
/* PR middle-end/83463 */
/* { dg-do compile } */
+/* { dg-prune-output "conflicting types for built-in" } */
/* { dg-options "-O2 -Wrestrict -Wno-pointer-to-int-cast" } */
int *a;
Index: gcc.dg/torture/pr55890-2.c
===
--- gcc.dg/torture/pr55890-2.c (revision 265727)
+++ gcc.dg/torture/pr55890-2.c (working copy)
@@ -1,4 +1,5 @@
/* { dg-do compile } */
+/* { dg-prune-output "conflicting types for built-in" } */
extern void *memcpy();
int main() { memcpy(); }
Index: gcc.dg/torture/pr55890-3.c
===
--- gcc.dg/torture/pr55890-3.c (revision 265727)
+++ gcc.dg/torture/pr55890-3.c (working copy)
@@ -1,4 +1,5 @@
/* { dg-do compile } */
+/* { dg-prune-output "conflicting types for built-in" } */
void *memmove ();
Index: gcc.dg/torture/pr71816.c
===
--- gcc.dg/torture/pr71816.c(revision 265727)
+++ gcc.dg/torture/pr71816.c(working copy)
@@ -1,4 +1,5 @@
/* { dg-do compile } */
+/* { dg-prune-output "conflicting types for built-in" } */
void *ext2fs_resize_mem_p;
struct ext2_icount_el {
[PATCH, testsuite] add "inf" target attribute
A number of test cases fail on pdp11 because they use the "inf" float value
which does not exist on that target (nor on VAX). Rainer Orth and Joseph Myers
suggested adding a new effective-target keyword to check for this, and require
it for tests that have that dependency.
The attached patch implements this. Ok for trunk?
paul
ChangeLog:
2018-11-01 Paul Koning
* doc/sourcebuild.texi (target attributes): Document new "inf"
effective target keyword.
Index: doc/sourcebuild.texi
===
--- doc/sourcebuild.texi(revision 265728)
+++ doc/sourcebuild.texi(working copy)
@@ -1393,8 +1393,10 @@ for any options added with @code{dg-add-options}.
Target has runtime support for any options added with
@code{dg-add-options} for any @code{_Float@var{n}} or
@code{_Float@var{n}x} type.
+
+@item inf
+Target supports floating point infinite (@code{inf}).
@end table
-
@subsubsection Fortran-specific attributes
@table @code
testsuite/ChangeLog:
2018-11-01 Paul Koning
* lib/target-supports.exp: Add check for "inf" effective target
keyword.
* gcc.dg/builtins-44.c: Skip if no infinite support.
* gcc.dg/builtins-45.c: Ditto.
* gcc.dg/torture/builtin-complex-1.c: Ditto.
* gcc.dg/torture/builtin-cproj-1.c: Ditto.
* gcc.dg/torture/builtin-frexp-1.c: Ditto.
* gcc.dg/torture/builtin-ldexp-1.c: Ditto.
* gcc.dg/torture/builtin-logb-1.c: Ditto.
* gcc.dg/torture/builtin-math-2.c: Ditto.
* gcc.dg/torture/builtin-math-5.c: Ditto.
* gcc.dg/torture/builtin-math-7.c: Ditto.
* gcc.dg/torture/builtin-modf-1.c: Ditto.
* gcc.dg/torture/type-generic-1.c: Ditto.
Index: lib/target-supports.exp
===
--- lib/target-supports.exp (revision 265728)
+++ lib/target-supports.exp (working copy)
@@ -8933,3 +8933,10 @@ proc check_effective_target_cet { } {
}
} "-O2" ]
}
+
+# Return 1 if target supports floating point "infinite"
+proc check_effective_target_inf { } {
+return [check_no_compiler_messages supports_inf assembly {
+const double pinf = __builtin_inf ();
+}]
+}
Index: gcc.dg/builtins-44.c
===
--- gcc.dg/builtins-44.c(revision 265728)
+++ gcc.dg/builtins-44.c(working copy)
@@ -1,4 +1,5 @@
/* { dg-do compile } */
+/* { dg-require-effective-target inf } */
/* { dg-options "-O1 -fno-trapping-math -fno-finite-math-only
-fdump-tree-optimized" } */
extern void f(int);
Index: gcc.dg/builtins-45.c
===
--- gcc.dg/builtins-45.c(revision 265728)
+++ gcc.dg/builtins-45.c(working copy)
@@ -1,4 +1,5 @@
/* { dg-do compile } */
+/* { dg-require-effective-target inf } */
/* { dg-options "-O1 -fno-trapping-math -fno-finite-math-only
-fdump-tree-optimized" } */
extern void f(int);
Index: gcc.dg/torture/builtin-complex-1.c
===
--- gcc.dg/torture/builtin-complex-1.c (revision 265728)
+++ gcc.dg/torture/builtin-complex-1.c (working copy)
@@ -1,6 +1,7 @@
/* Test __builtin_complex semantics. */
/* { dg-do run } */
/* { dg-options "-std=c11 -pedantic-errors" } */
+/* { dg-require-effective-target inf } */
/* { dg-add-options ieee } */
extern void exit (int);
Index: gcc.dg/torture/builtin-cproj-1.c
===
--- gcc.dg/torture/builtin-cproj-1.c(revision 265728)
+++ gcc.dg/torture/builtin-cproj-1.c(working copy)
@@ -7,6 +7,7 @@
/* { dg-do link } */
/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+/* { dg-require-effective-target inf } */
/* { dg-add-options ieee } */
/* All references to link_error should go away at compile-time. The
Index: gcc.dg/torture/builtin-frexp-1.c
===
--- gcc.dg/torture/builtin-frexp-1.c(revision 265728)
+++ gcc.dg/torture/builtin-frexp-1.c(working copy)
@@ -9,6 +9,7 @@
/* { dg-options "-fno-finite-math-only" { target sh*-*-* } } */
/* In order to fold algebraic exprs below, targets with "composite"
floating point formats need -funsafe-math-optimizations. */
+/* { dg-require-effective-target inf } */
/* { dg-options "-funsafe-math-optimizations" { target powerpc*-*-* } } */
extern void link_error(int);
Index: gcc.dg/torture/builtin-ldexp-1.c
===
--- gcc.dg/torture/builtin-ldexp-1.c(revision 265728)
+++ gcc.dg/torture/builtin-ldexp-1.c(working copy)
@@ -7,6 +7,7 @@
/* { dg-do link } */
/*
Re: [PATCH, testsuite] add "inf" target attribute
> On Nov 1, 2018, at 4:52 PM, Joseph Myers wrote:
>
> On Thu, 1 Nov 2018, Paul Koning wrote:
>
>> +@item inf
>> +Target supports floating point infinite (@code{inf}).
>> @end table
>
> Do you mean supports infinity for type double? (That's what the
> implementation does.) Supporting it for double is not the same as
> supporting it for float (SPU supports it for double but not float, hence
> various such tests, which require infinities or NaNs (or other IEEE
> features SPU doesn't have) for float, being disabled for SPU).)
Yes, I do mean for double. I suppose I should say that.
I noticed SPU avoids using __builtin_inff by conditional compile. I thought of
adding an "inff" flag also. The reason I didn't is that my target does not
need that, so I would end up with a new feature that would only be applied on
targets I do not know.
paul
Re: [PATCH, testsuite] test case fixes for pdp11
> On Nov 2, 2018, at 3:19 PM, Rainer Orth wrote: > > Hi Paul, > >> This patch fixes a number of test case failures on pdp11. Some are too >> large for the address space, some have dependencies on the float format that >> don't match the DEC format, some add pdp11 to the targets that expect >> particular compiler messages. > > unfortunately, even apart from the two bugs in your patch Andreas > already fixed, there are more problems: with the patch one gets 20 > warnings > > WARNING: compat.exp does not support dg-skip-if > > in mail-report.log for gcc.dg/compat. While the message is misleading > and it took me a moment to understand what's wrong, you should have > found this in your testing. A good way something like this doesn't go > unnoticed in a regtest is to run make mail-report.log in a vanilla and > patched tree and compare the output. Those WARNING and ERROR lines are > prominent there for a reason ;-) My apologies for these errors. I will strive to learn from the feedback you and Andreas have given and do it better next time. I wasn't ware of mail-report -- I have used contrib/test_summary in the past. What makes it more difficult in this case is that most of these test cases have not run in a long time (if ever) so the volume is quite large. I'm getting it down to a more manageable number, though 1000 out of 60k is still much higher than it should be. > While we give target maintainers quite a bit of leeway to apply > testsuite patches affecting only their targets, this needs to be > exercised with caution. Best test the modified testsuite on a different > target, too, to check that it doesn't break there. I understand, I will do so in the future. > Besides, a bit more detail on the failures you observe without your > patch would have been helpful. I noticed that some of the tests you > change already have dg-skip-if directives for avr with a comment of > "Program too big". It's hard to tell if this is the same issue as your > "limited code space". If so, it would be advisable and much more > expressive to introduce (yet another) effective-target keyword for this. I could use "! ptr32plus". I'm a bit hesitant to do so because I don't know what othe targets might match that. msp430, based on the comment, and possibly others. For tests that allocate megabyte buffers that's an obvious fit. For tests that generate large blocks of code it isn't quite so obvious; the gcc.gd/long_branch.c test is too big for pdp11 but it might be ok for other "small" targets. > The problem is that in the g??.dg/compat testsuites, dg keywords except > for dg-options are only supposed to go into the *_main.c file. The > following patch fixes this. Tested on i386-pc-solaris2.11 and > sparc-sun-solaris2.11, installed on mainline. Thank you. How can I tell for a particular test case or test directory what the rules are for where "dg" directives go, or which ones are supported? I know there is a fair amount of variability in this, but I don't yet understand what they all are. paul
Re: [PATCH, testsuite] ignore some "conflicting types for built-in" messages
> On Nov 3, 2018, at 10:12 PM, Jeff Law wrote: > > On 11/1/18 1:13 PM, Paul Koning wrote: >> A number of test cases contain declarations like: >> void *memcpy(); >> which currently are silently accepted on most platforms but not on all; >> pdp11 (and possibly some others) generate a "conflicting types for built-in >> function" warning. >> >> It was suggested to prune those messages because the test cases where these >> occur are not looking for the message but are testing some other issue, so >> the message is not relevant. The attached patch adds dg-prune-output >> directives to do so. >> >> Ok for trunk? >> >> paul >> >> ChangeLog: >> >> 2018-11-01 Paul Koning >> >> * gcc.dg/Walloca-16.c: Ignore conflicting types for built-in >> warnings. >> * gcc.dg/Wrestrict-4.c: Ditto. >> * gcc.dg/Wrestrict-5.c: Ditto. >> * gcc.dg/pr83463.c: Ditto. >> * gcc.dg/torture/pr55890-2.c: Ditto. >> * gcc.dg/torture/pr55890-3.c: Ditto. >> * gcc.dg/torture/pr71816.c: Ditto. > ISTM it'd be better to just fix memcpy to have a correct prototype. > > jeff I can do that, but I'm wondering if some systems have different prototypes than the C standard calls for so I'd end up breaking those. paul
Re: [PATCH, testsuite] ignore some "conflicting types for built-in" messages
> On Nov 5, 2018, at 11:45 AM, Jeff Law wrote: > >>> ... >> >> I can do that, but I'm wondering if some systems have different prototypes >> than the C standard calls for so I'd end up breaking those.I wouldn't worry >> about those. I think the bigger question (thanks > Martin) is whether or not any of those tests are checking for issues > that arise specifically due to not having a full prototype available > (and in those cases your fix is probably more appropriate). > > Probably the only way to figure that out is to dig into the history of > each one :( Mighty unpleasant. > > jeff I took a quick look. PR83655 is specifically about an issue due to a declaration with no prototype, but the others (55890, 71816, 83463, 83603, 84244) are not so clear to me. Still, what IS clear is that none of them are interested in messages that may or may not be generated as a result of these funny declarations. In other words, pruning the messages still looks appropriate. So where do I go from here? Without the change I can deal with this by recognizing these cases as false failures when I do my test runs. paul
Re: PR83750: CSE erf/erfc pair
> On Nov 5, 2018, at 1:48 PM, Michael Matz wrote: > > Hi, > > On Mon, 5 Nov 2018, Jeff Law wrote: > Don't we have a flag specific to honoring nans? Would that be better to use than flag_unsafe_math_optimizations? As Uli mentioned, there's >>> >>> That's only relevant for the comparison optimization, of course. >>> >>> Converting erfc to 1-erf is dubious, since the whole point of erfc is >>> for cases where 1-erf is inaccurate. (Conversion in the other >>> direction also needs flag_unsafe_math_optimizations.) >>> >> Understood. Thanks for clarifying. It seems like >> unsafe-math-optimization is a better fit than the nan specific flag. > > But still we should consider general usefullness, even with unsafe-math. > In this case we would remove a usage of a slow function that the user > specifically used to deal with inaccuracies with an equally slow function > (plus a little arithmetic that is shadows by the functions slowness) that > now exacly produces the inaccuracies the user wanted to avoid. I.e. the > speed gain is zero. The "canonicalization gain" referred to in the PR > might be real, but it comes at the cost of introducing definite > catastrophic cancellation. > > IMHO that's not a sensible transformation to do, under any flags. That seems right. The same goes for log vs. logp1, and exp vs. expm1. paul
Re: [PATCH, testsuite] add "inf" target attribute
> On Nov 4, 2018, at 2:33 PM, Jeff Law wrote:
>
> On 11/1/18 1:30 PM, Paul Koning wrote:
>> A number of test cases fail on pdp11 because they use the "inf" float value
>> which does not exist on that target (nor on VAX). Rainer Orth and Joseph
>> Myers suggested adding a new effective-target keyword to check for this, and
>> require it for tests that have that dependency.
>>
>> The attached patch implements this. Ok for trunk?
>>
>> paul
>>
>> ChangeLog:
>>
>> 2018-11-01 Paul Koning
>>
>> * doc/sourcebuild.texi (target attributes): Document new "inf"
>> effective target keyword.
> OK with me.
>
> jeff
Thanks. Committed, with the doc change clarified to address Joseph's comment.
paul
Index: doc/sourcebuild.texi
===
--- doc/sourcebuild.texi(revision 265814)
+++ doc/sourcebuild.texi(revision 265815)
@@ -1393,8 +1393,11 @@ for any options added with @code{dg-add-options}.
Target has runtime support for any options added with
@code{dg-add-options} for any @code{_Float@var{n}} or
@code{_Float@var{n}x} type.
+
+@item inf
+Target supports floating point infinite (@code{inf}) for type
+@code{double}.
@end table
-
@subsubsection Fortran-specific attributes
@table @code
[PATCH, pdp11] Bugfixes from test suite
This patch corrects a large number of test suite failures. I'm now down to about 1100 failures out of over 60k total, from at least 4000 before. Committed. paul ChangeLog: 2018-11-08 Paul Koning * config/pdp11/constraints.md: Add "Z" series constraints for use with pre-dec and post-inc addressing. * config/pdp11/pdp11-protos.m (expand_block_move): Delete. (pdp11_expand_operands): Add int argument (word count). (pdp11_sp_frame_offset): Delete. (pdp11_cmp_length): New function. (pushpop_regeq): New function. * config/pdp11/pdp11.c (TARGET_STACK_PROTECT_RUNTIME_ENABLED_P): Add hook. (pdp11_expand_prologue, pdp11_expand_epilogue): Rewrite for new frame layout. (pdp11_initial_elimination_offset): Ditto. (pdp11_expand_operands): Add word count argument. Bugfixes. (output_move_multiple): Change how pointer adjustment is done. (pdp11_gen_int_label): Correct format. (output_ascii): Ditto. (pdp11_asm_output_var): Add code for DEC assembler case. (pdp11_asm_print_operand): Bugfix for CONST_DOUBLE holding integer value. (legitimate_const_double_p): Ditto. (pdp11_register_move_cost): Adjust for new register classes. (pdp11_regno_reg_class): Ditto. (expand_block_move): Delete. (pushpop_regeq): New function. (pdp11_legitimate_address_p): Bugfix in check for constant offset. (pdp11_sp_frame_offset): Delete. (pdp11_reg_save_size): New helper function for new frame layout. (output_addr_const_pdp11): Remove CONST_DOUBLE case. (pdp11_expand_shift): Bugfix in check for constant shift count. (pdp11_shift_length): Ditto. (pdp11_assemble_shift): Copy input to pdp11_expand_operands. (pdp11_cmp_length): New function. * config/pdp11/pdp11.h (TARGET_CPU_CPP_BUILTINS): Add macros for some compile options. (FIXED_REGISTERS): Remove HARD_FRAME_POINTER_REGNUM. (CALL_USED_REGISTERS): Ditto. (ELIMINABLE_REGS): Ditto. (REGISTER_NAMES): Ditto. (reg_class): Add classes NOTR0_REG through NOTSP_REG for use by Z constraints. (REG_CLASS_NAMES): Ditto. (REG_CLASS_CONTENTS): Ditto. Also remove HARD_FRAME_POINTER_REGNUM. (CPU_REG_CLASS): New macro. (CLASS_MAX_NREGS): Adjust for new register classes. (FUNCTION_PROFILER): Make no-op. (may_call_alloca): Remove unused declaration. (ASM_OUTPUT_ALIGN): Add workaround for PR87795. (ASM_OUTPUT_SKIP): Fix format. * config/pdp11/pdp11.md (unspecv): Add UNSPECV_MOVMEM. (HARD_FRAME_POINTER_REGNUM): Remove. (return): Delete. (*rts): Rename. Remove epilogue related checks. (cmpsi, cmpdi): New insn. (cbranch4): Change to apply to SI and DI modes as well. (mov): Change constraints to enforce that push/pop destination cannot use the same register as source. (*mov): Ditto. (movmemhi, movmemhi1, movmemhi_nocc): Change to expand block move at assembly output rather than as RTL expander. (zero_extendqihi2): Bugfix in check for same registers. (adddi3_nocc): Bugfix in check for constant operand. (addsi3_nocc): Ditto. (subdi3_nocc): Ditto. (subsi3_nocc): Ditto. (negdi2_nocc): Copy input to pdp11_expand_operands. (negsi2_nocc): Ditto. (bswap2_nocc): Ditto. * config/pdp11/pdp11.opt (mlra): Fix documentation. * config/pdp11/t-pdp11: Use -Os. Index: config/pdp11/constraints.md === --- config/pdp11/constraints.md (revision 265931) +++ config/pdp11/constraints.md (working copy) @@ -88,3 +88,32 @@ (match_test "memory_address_p (GET_MODE (op), XEXP (op, 0)) && no_side_effect_operand (op, GET_MODE (op))"))) +;; What follows is a set of constraints used to prevent the generation +;; of insns that have a register as source, and an auto-increment or +;; auto-decrement memory reference as the destination where the register +;; is the same as the source. On the PDP11, such instructions are not +;; implemented consistently across the models and often do something +;; different from what the RTL intends. +(define_register_constraint "Z0" "NOTR0_REG" "Register other than 0") +(define_register_constraint "Z1" "NOTR1_REG" "Register other than 1") +(define_register_constraint "Z2" "NOTR2_REG" "Register other than 2") +(define_register_constraint "Z3" "NOTR3_REG" "Register other than 3") +(define_register_constraint "Z4" "NOTR4_REG" "Register other than 4") +(define_register_constrain
Ping: [PATCH, testsuite] ignore some "conflicting types for built-in" messages
Ping.
I'd like to commit this. The discussion seems to have ended up with the
conclusion that this is a reasonable approach.
paul
> On Nov 1, 2018, at 3:13 PM, Paul Koning wrote:
>
> A number of test cases contain declarations like:
> void *memcpy();
> which currently are silently accepted on most platforms but not on all; pdp11
> (and possibly some others) generate a "conflicting types for built-in
> function" warning.
>
> It was suggested to prune those messages because the test cases where these
> occur are not looking for the message but are testing some other issue, so
> the message is not relevant. The attached patch adds dg-prune-output
> directives to do so.
>
> Ok for trunk?
>
> paul
>
> ChangeLog:
>
> 2018-11-01 Paul Koning
>
> * gcc.dg/Walloca-16.c: Ignore conflicting types for built-in
> warnings.
> * gcc.dg/Wrestrict-4.c: Ditto.
> * gcc.dg/Wrestrict-5.c: Ditto.
> * gcc.dg/pr83463.c: Ditto.
> * gcc.dg/torture/pr55890-2.c: Ditto.
> * gcc.dg/torture/pr55890-3.c: Ditto.
> * gcc.dg/torture/pr71816.c: Ditto.
>
> Index: gcc.dg/Walloca-16.c
> ===
> --- gcc.dg/Walloca-16.c (revision 265727)
> +++ gcc.dg/Walloca-16.c (working copy)
> @@ -1,5 +1,6 @@
> /* PR tree-optimization/84224 */
> /* { dg-do compile } */
> +/* { dg-prune-output "conflicting types for built-in" } */
> /* { dg-options "-O0 -Walloca" } */
>
> void *alloca ();
> Index: gcc.dg/Wrestrict-4.c
> ===
> --- gcc.dg/Wrestrict-4.c (revision 265727)
> +++ gcc.dg/Wrestrict-4.c (working copy)
> @@ -3,6 +3,7 @@
>Test to verify that invalid calls to built-in functions declared
>without a prototype don't cause an ICE.
>{ dg-do compile }
> + { dg-prune-output "conflicting types for built-in" }
>{ dg-options "-O2 -Warray-bounds -Wrestrict" } */
>
> void* memcpy ();
> Index: gcc.dg/Wrestrict-5.c
> ===
> --- gcc.dg/Wrestrict-5.c (revision 265727)
> +++ gcc.dg/Wrestrict-5.c (working copy)
> @@ -2,6 +2,7 @@
>functions declared with no prototype are checked for overlap, and that
>invalid calls are ignored.
> { dg-do compile }
> + { dg-prune-output "conflicting types for built-in" }
> { dg-options "-O2 -Wrestrict" } */
>
> typedef __SIZE_TYPE__ size_t;
> Index: gcc.dg/pr83463.c
> ===
> --- gcc.dg/pr83463.c (revision 265727)
> +++ gcc.dg/pr83463.c (working copy)
> @@ -1,5 +1,6 @@
> /* PR middle-end/83463 */
> /* { dg-do compile } */
> +/* { dg-prune-output "conflicting types for built-in" } */
> /* { dg-options "-O2 -Wrestrict -Wno-pointer-to-int-cast" } */
>
> int *a;
> Index: gcc.dg/torture/pr55890-2.c
> ===
> --- gcc.dg/torture/pr55890-2.c(revision 265727)
> +++ gcc.dg/torture/pr55890-2.c(working copy)
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-prune-output "conflicting types for built-in" } */
>
> extern void *memcpy();
> int main() { memcpy(); }
> Index: gcc.dg/torture/pr55890-3.c
> ===
> --- gcc.dg/torture/pr55890-3.c(revision 265727)
> +++ gcc.dg/torture/pr55890-3.c(working copy)
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-prune-output "conflicting types for built-in" } */
>
> void *memmove ();
>
> Index: gcc.dg/torture/pr71816.c
> ===
> --- gcc.dg/torture/pr71816.c (revision 265727)
> +++ gcc.dg/torture/pr71816.c (working copy)
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-prune-output "conflicting types for built-in" } */
>
> void *ext2fs_resize_mem_p;
> struct ext2_icount_el {
>
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
> On Nov 14, 2018, at 10:44 AM, Jozef Lawrynowicz
> wrote:
>
> Patch 1 tweaks dg directives in tests specifically for msp430. Many of
> these are extensions to existing target selectors in dg directives.
>
> <0001-TESTSUITE-MSP430-Tweak-dg-directives-for-msp430-elf.patch>
For pr41779.c, you have
+/* { dg-skip-if "int is smaller than float" { msp430-*-* } } */
I take it that means: sizeof(int) < sizeof(float). That property also holds
for pdp11 and perhaps other targets. Would it make sense to introduce a new
effective-target flag for that check instead?
paul
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
> On Nov 14, 2018, at 1:00 PM, Jozef Lawrynowicz
> wrote:
>
> On 14/11/2018 16:54, Andreas Schwab wrote:
>> On Nov 14 2018, Jozef Lawrynowicz wrote:
>>
>>> diff --git a/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
>>> b/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
>>> index 6b1c427..71d24ce 100644
>>> --- a/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
>>> +++ b/gcc/testsuite/c-c++-common/torture/builtin-arith-overflow-10.c
>>> @@ -1,6 +1,7 @@
>>> /* Test __builtin_{add,sub}_overflow on {,un}signed long int. */
>>> /* { dg-do run } */
>>> /* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
>>> +/* { dg-timeout 120 { target msp430-*-* } } */
>> Are you sure you want to _decrease_ the timeout? The default is 300
>> seconds.
>>
>> Andreas.
>>
> The timeout as set in the dejagnu configuration for msp430
> ([dejagnu.git]/baseboards/msp430-sim.exp) is 30, which is rarely hit. There
> are some tests which pass most of of the time but will occasionally timeout
> so maybe the default timeout in dejagnu is worth increasing a little as well.
Would it make sense to use { dg-timeout-factor 4 ... } instead? That would
make it explicit that you're raising rather than lowering the timeout.
paul
Re: [PATCH 1/7][MSP430][TESTSUITE] Tweak dg-directives for msp430-elf
> On Nov 14, 2018, at 5:19 PM, Jozef Lawrynowicz
> wrote:
>
> On Wed, 14 Nov 2018 11:30:39 -0500
> Paul Koning wrote:
>
>>> On Nov 14, 2018, at 10:44 AM, Jozef Lawrynowicz
>>> wrote:
>>>
>>> Patch 1 tweaks dg directives in tests specifically for msp430. Many of
>>> these are extensions to existing target selectors in dg directives.
>>>
>>> <0001-TESTSUITE-MSP430-Tweak-dg-directives-for-msp430-elf.patch>
>>
>> For pr41779.c, you have
>>
>> +/* { dg-skip-if "int is smaller than float" { msp430-*-* } } */
>>
>> I take it that means: sizeof(int) < sizeof(float). That property also holds
>> for pdp11 and perhaps other targets. Would it make sense to introduce a new
>> effective-target flag for that check instead?
>>
>> paul
>>
>
> Paul,
>
> Yes you are correct the comment implies sizeof(int) < sizeof(float).
>
> I believe this was the only test where this property affected the test
> results, so a new effective-target flag is probably only worth adding if it
> affects at least a couple of tests.
> On the other hand, I suppose there is no harm in adding another
> check-effective-target and it at least means we'll catch failures across more
> targets.
>
> I'd be curious if the line I added the xfail to in c-c++-common/pr57371-2.c
> also fails for pdp11.
>
> The conversion to float might be getting optimized out whenever
> sizeof(int) < sizeof(float).
>
> Thanks,
> Jozef
Yes, that test on pr57371-2.c also fails on pdp11.
paul
[PATCH, testsuite] indicate no "weak" support in pdp11
This patch changes check_weak_available to report that pdp11 does not support
"weak". A number of test case failures are caused by attempts to use weak
support which is missing in the pdp11 flavor of a.out.
I'm not sure if this is covered by target maintainer authority so I figure it's
best to ask for approval.
Ok for trunk?
paul
testsuite/ChangeLog:
2018-11-19 Paul Koning
* lib/target-supports.exp (check_weak_available): Return "no" for
pdp11.
Index: lib/target-supports.exp
===
--- lib/target-supports.exp (revision 266288)
+++ lib/target-supports.exp (working copy)
@@ -314,6 +314,12 @@ proc check_weak_available { } {
return 1
}
+# pdp11 doesn't support it
+
+if { [istarget pdp11*-*-*] } {
+ return 0
+}
+
# ELF and ECOFF support it. a.out does with gas/gld but may also with
# other linkers, so we should try it
Re: [PATCH, testsuite] indicate no "weak" support in pdp11
> On Nov 19, 2018, at 5:20 PM, Jeff Law wrote: > > On 11/19/18 3:18 PM, Paul Koning wrote: >> This patch changes check_weak_available to report that pdp11 does not >> support "weak". A number of test case failures are caused by attempts to >> use weak support which is missing in the pdp11 flavor of a.out. >> >> I'm not sure if this is covered by target maintainer authority so I figure >> it's best to ask for approval. >> >> Ok for trunk? >> >> paul >> >> testsuite/ChangeLog: >> >> 2018-11-19 Paul Koning >> >> * lib/target-supports.exp (check_weak_available): Return "no" for >> pdp11. > Yes. And FWIW this kind of change falls into what maintainers can make > and self-approve. > > jeff Thanks for the clarification. Committed. paul
[PATCH, testsuite] Fix pdp11 test failures
This corrects a number of pdp11 test suite failures. The first set fail due to
larger than supported alignment. The next two look for .ascii directives in
the assembly output which pdp11 does not use. The last one needs adjustment
because CSE loads subroutine addresses into registers in this test.
Committed.
paul
testsuite/ChangeLog:
2018-11-19 Paul Koning
* gcc.c-torture/execute/align-3.c: Skip if pdp11.
* gcc.c-torture/execute/pr23467.c: Ditto.
* gcc.c-torture/execute/pr36093.c: Ditto.
* gcc.c-torture/execute/pr43783.c: Ditto.
* gcc.dg/const-elim-2.c: Xfail if pdp11.
* gcc.dg/torture/pr36400.c: Ditto.
* gcc.dg/tree-ssa/loop-1.c: Xfail for pdp11. Add pdp11 to check
for jsr.
Index: gcc.c-torture/execute/align-3.c
===
--- gcc.c-torture/execute/align-3.c (revision 266295)
+++ gcc.c-torture/execute/align-3.c (working copy)
@@ -1,3 +1,5 @@
+/* { dg-skip-if "small alignment" { pdp11-*-* } } */
+
void func(void) __attribute__((aligned(256)));
void func(void)
Index: gcc.c-torture/execute/pr23467.c
===
--- gcc.c-torture/execute/pr23467.c (revision 266295)
+++ gcc.c-torture/execute/pr23467.c (working copy)
@@ -1,3 +1,5 @@
+/* { dg-skip-if "small alignment" { pdp11-*-* } } */
+
struct s1
{
int __attribute__ ((aligned (8))) a;
Index: gcc.c-torture/execute/pr36093.c
===
--- gcc.c-torture/execute/pr36093.c (revision 266295)
+++ gcc.c-torture/execute/pr36093.c (working copy)
@@ -1,3 +1,5 @@
+/* { dg-skip-if "small alignment" { pdp11-*-* } } */
+
extern void abort (void);
typedef struct Bar {
Index: gcc.c-torture/execute/pr43783.c
===
--- gcc.c-torture/execute/pr43783.c (revision 266295)
+++ gcc.c-torture/execute/pr43783.c (working copy)
@@ -1,3 +1,5 @@
+/* { dg-skip-if "small alignment" { pdp11-*-* } } */
+
typedef __attribute__((aligned(16)))
struct {
unsigned long long w[3];
Index: gcc.dg/const-elim-2.c
===
--- gcc.dg/const-elim-2.c (revision 266295)
+++ gcc.dg/const-elim-2.c (working copy)
@@ -1,7 +1,7 @@
/* The string constant in this test case should be emitted exactly once. */
/* { dg-do compile } */
/* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-times "hi there" 1 { xfail nvptx-*-* } } } */
+/* { dg-final { scan-assembler-times "hi there" 1 { xfail nvptx-*-* pdp11-*-*
} } } */
static inline int returns_23() { return 23; }
Index: gcc.dg/torture/pr36400.c
===
--- gcc.dg/torture/pr36400.c(revision 266295)
+++ gcc.dg/torture/pr36400.c(working copy)
@@ -14,4 +14,4 @@ void baz()
barptr->some_string = "Everything OK";
}
-/* { dg-final { scan-assembler "Everything OK" { xfail nvptx-*-* } } } */
+/* { dg-final { scan-assembler "Everything OK" { xfail nvptx-*-* pdp11-*-* } }
} */
Index: gcc.dg/tree-ssa/loop-1.c
===
--- gcc.dg/tree-ssa/loop-1.c(revision 266295)
+++ gcc.dg/tree-ssa/loop-1.c(working copy)
@@ -46,7 +46,7 @@ int xxx(void)
/* CRIS keeps the address in a register. */
/* m68k sometimes puts the address in a register, depending on CPU and PIC. */
-/* { dg-final { scan-assembler-times "foo" 5 { xfail hppa*-*-* ia64*-*-*
sh*-*-* cris-*-* crisv32-*-* fido-*-* m68k-*-* i?86-*-mingw* i?86-*-cygwin*
x86_64-*-mingw* visium-*-* nvptx*-*-* } } } */
+/* { dg-final { scan-assembler-times "foo" 5 { xfail hppa*-*-* ia64*-*-*
sh*-*-* cris-*-* crisv32-*-* fido-*-* m68k-*-* i?86-*-mingw* i?86-*-cygwin*
x86_64-*-mingw* visium-*-* nvptx*-*-* pdp11*-*-* } } } */
/* { dg-final { scan-assembler-times "foo,%r" 5 { target hppa*-*-* } } } */
/* { dg-final { scan-assembler-times "= foo" 5 { target ia64*-*-* } } } */
/* { dg-final { scan-assembler-times "call\[ \t\]*_foo" 5 { target
i?86-*-mingw* i?86-*-cygwin* } } } */
@@ -53,6 +53,6 @@ int xxx(void)
/* { dg-final { scan-assembler-times "call\[ \t\]*foo" 5 { target
x86_64-*-mingw* } } } */
/* { dg-final { scan-assembler-times "jsr|bsrf|blink\ttr?,r18" 5 { target
sh*-*-* } } } */
/* { dg-final { scan-assembler-times "Jsr \\\$r" 5 { target cris-*-* } } } */
-/* { dg-final { scan-assembler-times "\[jb\]sr" 5 { target fido-*-* m68k-*-* }
} } */
+/* { dg-final { scan-assembler-times "\[jb\]sr" 5 { target fido-*-* m68k-*-*
pdp11-*-* } } } */
/* { dg-final { scan-assembler-times "bra *tr,r\[1-9\]*,r21" 5 { target
visium-*-* } } } */
/* { dg-final { scan-assembler-times "(?n)\[ \t\]call\[ \t\].*\[ \t\]foo," 5 {
target nvptx*-*-* } } } */
[PATCH, pdp11] Fix 64 bit divide
This fixes a number of testsuite failures in pdp11. Committed. paul ChangeLog: 2018-11-25 Paul Koning * config/pdp11/pdp11.h (TARGET_HAS_NO_HW_DIVIDE): Define. Index: config/pdp11/pdp11.h === --- config/pdp11/pdp11.h(revision 266438) +++ config/pdp11/pdp11.h(working copy) @@ -143,6 +143,11 @@ extern const struct real_format pdp11_d_format; /* Define this if move instructions will actually fail to work when given unaligned data. */ #define STRICT_ALIGNMENT 1 + +/* "HW_DIVIDE" actually means 64 by 32 bit divide. While some PDP11 + models have hardware divide, it is for 32 by 16 bits only, so we + call this platform "no hardware divide". */ +#define TARGET_HAS_NO_HW_DIVIDE 1 /* Standard register usage. */
Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.
> On Nov 26, 2018, at 4:13 AM, Mark Wielaard wrote: > > With -Wtrampolines a warning is produced whenever gcc generates executable > code on the stack at runtime to support taking a nested function address > that is used to call the nested function indirectly when it needs to access > any variables in its lexical scope. > > As a result the stack has to be marked as executable even for targets which > have a non-executable stack as default. > > Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those targets > that have a non-executable default stack based on when they call > file_end_indicate_exec_stack. The word "default" suggests this is a constant attribute of the platform. Can it be variable? Whether the stack is executable or not may depend on -m switches. paul
