"Sebastian Perta" writes:
> * config/rl78/rl78.md: New define_expand "umaxdi3".
> * config/rl78/rl78.md: New define_expand "smaxdi3".
> * config/rl78/rl78.md: New define_expand "smindi3".
> * config/rl78/rl78.md: New define_expand "umindi3".
> * config/rl78/rl78.md:
Jeff Law writes:
> So I think you're ultimately far better off determining why GCC does not
> generate efficient code for 64bit logicals on the rl78 target.
In thinking about this more, one possible reason is that rl78 has an
8-bit WORD_MODE. Which means DImode operations are not reduced to
SIm
This is OK.
I wonder if these types of optimizations should be added to the
assembler too? At least, if relaxation is enabled...
If the RX and RL78 now share interrupt/vector semantics, can we combine
the docs? I.e. instead of a new section for RL78, can we change the RX
section to say something like "For RX and RL78..." ?
Sebastian Perta writes:
> I've updated the patch (extend.texi) as you suggested.
> Please let me know if this is OK to check-in, thank you!
Looks OK to me, but wait a day or two for a docs person to comment on...
> -On RX targets, you may specify one or more vector numbers as arguments
> +On RX
The msp43-specific parts look OK to me, but obviously they're kinda
useless without the core changes :-)
"Sebastian Perta" writes:
>>>Looks OK to me, but wait a day or two for a docs person to comment on...
> 6 days no comments so far, can I check in now?
Yup, go ahead.
>>>if the new line is too long
> There are many other lines which have the same length or are even longer
> this is why I let it a
This is OK. In the future, please include the Changelog entry as a
separate text, not part of the patch, as it will rarely apply cleanly.
"Sebastian Perta" writes:
>
> * config/rl78/rl78.md (movdf): New define expand.
Const type promotion is the bane of embedded developers...
One thing to try is to use (subreg:QI in a define_expand, so that
there's a one_cmplhi2 pattern that expands to two QImode insns that
operate on HImode input/outputs via SUBREGs.
I don't have high hopes of gcc optimizing this properly in
"Sebastian Perta" writes:
> Is this similar to what you had in mind?
Yes. Did it affect code size in any of the larger tests? I was hoping
that it wouldn't force too much into 8-bit registers and cause more
moves to be needed elsewhere.
(and even if it didn't, I think this one feels "more corr
"H.J. Lu" writes:
> config/
>
> * plugins.m4 (AC_PLUGINS): Use dlsym to check if libdl is needed.
This is OK.
The testsuite parts are OK with me, but the tree.c change needs
separate approval...
Alexander Monakov writes:
> I'm not sure. It has a weaker contract compared to qsort, and I believe
> functions in libiberty are understood to provide stronger/better replacements.
The original intent of libiberty was to provide a stronger *portability*
contract, i.e. to work around differences
> As for PSImode, I dunno - seems only m32c and AVR use that? I have no
> way how to perform testing on such targets.
and msp430. m32c and msp430 have simulators in gdb.
Responding wrt "DJ's proposed changes":
> > As to what bits are modified, that's target dependent as the
> > precise size of the partial modes is target dependent.
>
> GET_MODE_PRECISION (mode) is the precision of all modes.
True, but not all the compiler uses that info when it should. There
a
> > There's still lots of places in gcc that use SIZE where they should
> > use PRECISION.
>
> Yes, and in time, they’ll all get cleaned up.
Hopefully not much time, if I can get my patches commit-worthy. I'm
actually debugging a pointer-signed-math regression now.
> > I also added a step that
> Ah, cool, I didn't know all that was in.
Well, the precision part is in, but the __intN part isn't yet. Each
time I do a "final" check for regressions, something new has been
added to gcc which breaks it all again... :-P
The following five patches are the latest in my ongoing work to
replace the hard-coded __int128 type with a more flexible __int
system that allows target-specific types that correspond to
partial-int modes. Specifically, this will allow targets to have
pointers that aren't powers-of-two in size.
This patch is part of the __intN series, but is independent. It
provides an additional optimization opportunity, since the MSP430 does
a lot of conversions between HImode and PSImode.
* expr.c (convert_move): If the target has an explicit converter,
use it.
Index: gcc/expr.c
===
The purpose of this set of changes is to remove assumptions in GCC
about type sizes. Previous to this patch, GCC assumed that all types
were powers-of-two in size, and used naive math accordingly.
Old:
POINTER_SIZE / BITS_PER_UNIT
TYPE_SIZE
GET_MODE_BITSIZE
New:
This is the MSP430-specific use of the new intN framework to enable
true 20-bit pointers. Since I'm one of the MSP430 maintainers, this
patch is being posted for reference, not for approval.
gcc/config/msp430
* config/msp430/msp430-modes.def (PSI): Add.
* config/msp430/msp430-pr
Changes to the testsuite to make tests more portable to targets with
unusual address spaces.
* testsuite/
* lib/target-supports.exp (check_effective_target_size32plus):
Increase size to avoid false positives on 24-bit address spaces.
* gcc.c-torture/compile/limits-stringli
> A while ago I've removed a couple of those 'typedef struct' things, as
> they are not required in C++ anymore. Is there any particular reason
> why this couldn't be simply 'struct int_n_data_t' ?
No particular reason.
Minor tweak, committed.
* config/rl78/rl78-virt.md (movhi_virt): Allow const->far moves.
Index: config/rl78/rl78-virt.md
===
--- config/rl78/rl78-virt.md(revision 213994)
+++ config/rl78/rl78-virt.md(working copy)
@@
The G10 family doesn't support this opcode. Committed.
* config/rl78/rl78-expand.md (umulqihi3): Disable for G10.
* config/rl78/rl78-virt.md (umulhi3_shift_virt): Likewise.
(umulqihi3_virt): Likewise.
* config/rl78/rl78-real.md (umulhi3_shift_real): Likewise.
Letting GCC think that any mem-mem alternative is OK leads to trouble
with far mem to far mem moves, so split out the moves we can make.
Committed.
* config/rl78/predicates.md (rl78_near_mem_operand): New.
* config/rl78/rl78-virt.md (movqi_virt_mm, movqi_virt)
(movhi_virt_
Minor optimization. Committed.
* config/rl78/rl78-real.md (addqi3_real): Allow adding global
variables to the accumulator.
Index: config/rl78/rl78-real.md
===
--- config/rl78/rl78-real.md(revision 213996)
+++ co
> Why are types only entered in integer_types if wider than long long?
IIRC it was so that TImode (__int128) could get into the array (it was
there before) without adding the other smaller types, which (I think)
don't need to be in there. I don't recall why they're left out,
but... ah, I remembe
> I don't see any corresponding HOST_BITS_PER_WIDE_INT test for __int128
> being removed (and anyway HOST_BITS_PER_WIDE_INT is now always 64, so such
> a test for __int128 would be dead code).
It was there when I started the patch, honest! :-)
Removed ;-)
> > For each __int we need to provid
> Maybe you need to refactor __glibcxx_digits so there is a version taking
> the bitsize as an argument rather than using sizeof(T) * __CHAR_BIT__, but
> that should be the only change needed to handle such types with the
> existing macros. The bitsize macros should be the only ones needing
>
> > > Maybe you need to refactor __glibcxx_digits so there is a version taking
> > > the bitsize as an argument rather than using sizeof(T) * __CHAR_BIT__,
> > > but
> > > that should be the only change needed to handle such types with the
> > > existing macros. The bitsize macros should be t
> I don't see flag_iso as relevant here (since the macros are in the
> implementation namespace). The definitions could reasonably be restricted
> to c_dialect_cxx (), though, given that they are specifically for use by
> libstdc++ (and it's easier to add a macro later for C if needed, than to
> This patch should fix it. Okay to apply?
Ok. Thanks!
> 2014-08-28 Jan-Benedict Glaw
>
> * config/mep/mep-pragma.c (mep_pragma_coprocessor_subclass): Rework
> to silence warning.
> * config/mep/mep.c (VECTOR_TYPE_P): Remove duplicate definition.
> On 08/13/14 16:10, DJ Delorie wrote:
> > The purpose of this set of changes is to remove assumptions in GCC
> > about type sizes. Previous to this patch, GCC assumed that all types
> > were powers-of-two in size, and used naive math accordingly.
> >
&g
> > * testsuite/
> > * lib/target-supports.exp (check_effective_target_size32plus):
> > Increase size to avoid false positives on 24-bit address spaces.
> > * gcc.c-torture/compile/limits-stringlit.c: Skip if msp430.
> > * gcc.dg/lto/pr54709_1.c: Fix memcpy prototype.
> > * gcc
> > * expr.c (convert_move): If the target has an explicit converter,
> > use it.
> OK.
Thanks! Committed.
> > gcc/
> > * cppbuiltin.c (define_builtin_macros_for_type_sizes): Round
> > pointer size up to a power of two.
> > * defaults.h (DWARF2_ADDR_SIZE): Round up.
> > (POINTER_SIZE_UNITS): New, rounded up value.
> > * dwarf2asm.c (size_of_encoded_value): Use it.
> > (dw2_outpu
Minor bit of docs for an msp430 option. OK for trunk and 4.9 branch?
* doc/invoke.texi (MSP430 Options): Add -minrt.
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 214976)
+++ doc/invoke.texi (working copy
> > Minor bit of docs for an msp430 option. OK for trunk and 4.9 branch?
>
> Ok.
>
> > * doc/invoke.texi (MSP430 Options): Add -minrt.
Thanks! Committed.
Consider this source:
extern char *index(const char *,int);
static int index;
"index" is a builtin as well, but because it's a builtin gcc skips the
"previous declaration was here..." despite having *a* previous decl it
could complain about. Note that newlib provides decls for many
builtins
> Please send a patch that adds a testcase to the testsuite to show the
> diagnostics you get after the patch.
How's this?
* c-decl.c (locate_old_decl): If a previous conflicting decl is
both explicit and builtin, print the location of the explicit one.
* gcc.dg/mismatc
Thanks! Committed.
> This patch just changes a QImode (const_int 255) to (const_int -1),
> since the canonical form is to sign-extend. As things stand the pattern
> trips a new assert added on the wide-int branch.
Ok. Thanks!
All RX opcodes which take a dsp:8 also take a dsp:16 so we can relax
this offset check. Committed.
* config/rx/rx.c (rx_mode_dependent_address_p): Allow offsets up
to 16 bits.
Index: config/rx/rx.c
===
--- config/rx
> * config/m32c/cond.md (stzx_16): Use register_operand for operand 0.
> (stzx_24_): Likewise mra_operand.
Ok. Thanks!
> > if (same_type_p (TYPE_MAIN_VARIANT (t1), long_long_unsigned_type_node)
> > || same_type_p (TYPE_MAIN_VARIANT (t2),
> > long_long_unsigned_type_node))
> > return build_type_attribute_variant (long_long_unsigned_type_node,
> >
This is what I ended up with for the test case. It was a bit tricky
since it only works with msp430x (not msp430) and requires the gnu
extensions. Is this OK? If so, is there anything else, or can I
check the whole mess in yet?
// { dg-do compile { target msp430*-*-* } }
// { dg-options "-std
> > extensions. Is this OK? If so, is there anything else, or can I
> > check the whole mess in yet?
>
> Go ahead.
Thanks! Committed.
> ../../gcc/gcc/config/rs6000/rs6000-c.c:237:24: error: ‘RID_INT128’ was not
> declared in this scope
Two options:
1. If you know the RS6000 will never have any __intN other than
__int128, just use RID_INT_N_0, although this is a hack it will
work as long as there *is* an __int128 for RS6
> This is the MSP430-specific use of the new intN framework to enable
> true 20-bit pointers. Since I'm one of the MSP430 maintainers, this
> patch is being posted for reference, not for approval.
Now that the other parts are committed, I'm checking this one in too.
> gcc/config/msp430
>
As long as you commit to gcc first, or at the same time as other
commits, you'll be fine. If you commit elsewhere first or only, it
may be reverted by a script.
> If you could implement the second option, it would be appreciated.
Could you please test this for me? It builds as a powerpc-elf
cross-compiler (at least the host half) but I don't have a power
machine here to test on.
Index: rs6000-c.c
In the event that pointer sizes aren't powers of two, choose a more
suitable alignment than (unsigned)(-1), which results in HUGE file
sizes. Ok?
Index: gcc/lto/lto-object.c
===
--- gcc/lto/lto-object.c(revision 216287)
+++
Since flag-types.h is used in target code, it needs to be -Wall-safe
for targets with 16-bit "int". OK?
Index: gcc/flag-types.h
===
--- gcc/flag-types.h(revision 216287)
+++ gcc/flag-types.h(working copy)
@@ -229,17 +229,17
A few more cases where pointers were assumed to be whole bytes.
Ok?
Index: gcc/c-family/c-cppbuiltin.c
===
--- gcc/c-family/c-cppbuiltin.c (revision 216287)
+++ gcc/c-family/c-cppbuiltin.c (working copy)
@@ -668,13 +668,13 @@ cpp_ato
> I don't see what the stor-layout.c changes have to do with that
> description, or why they are correct (they look wrong to me; the
> existing addition of BITS_PER_UNIT_LOG + 1 looks logically correct
> for bitsizetype).
sooo... the type for bitsizetype will *always* be a bigger type than
sizety
> The other changes are OK.
Committed those then. Thanks!
> This is okay.
Thanks! Committed.
> With a ChangeLog entry yes.
Thanks! Committed.
> > In the event that pointer sizes aren't powers of two, choose a more
> > suitable alignment than (unsigned)(-1), which results in HUGE file
> > sizes. Ok?
>
> Ok.
Thanks! Committed.
> FAIL: g++.dg/init/enum1.C -std=gnu++11 (test for errors, line 12)
> FAIL: g++.dg/init/enum1.C -std=gnu++1y (test for errors, line 12)
> FAIL: g++.dg/init/enum1.C -std=gnu++98 (test for errors, line 12)
>
> That used to complain about "enum1.C:12:1: error: no integral type can
> represent
Fixes BISX.A bug. Committed.
2014-10-23 DJ Delorie
* config/msp430/msp430.c (msp430_print_operand): 'x' modifier is
independend of -mlarge.
* config/msp430/constraints.md (Ys): Update comment.
Index: config/msp430/cons
Fixed PR/63582. Tested with no regressions on x86-64 and ix86. Ok?
* tree.c (build_common_tree_nodes): Don't even store the
__int128 types if they're not supported.
Index: tree.c
===
--- tree.c (revision 21667
> * config/mep/mep.c: Include rtl-iter.h.
> (global_reg_mentioned_p_1): Take a const_rtx and return a bool.
> (xtensa_tls_referenced_p): Return a bool. Use FOR_EACH_SUBRTX.
OK.
> * config/mep/mep.c (mep_store_find_set): Take a const_rtx and
> return a bool. Replace "void *" with specific type. Iterate
> over all subrtxes.
> (mep_store_data_bypass_1): Update calls accordingly.
OK.
> gcc/
> * config/mep/mep.c (mep_mul_hilo_bypass_1): Delete.
> (mep_mul_hilo_bypass_p): Use FOR_EACH_SUBRTX.
OK.
> > Fixed PR/63582. Tested with no regressions on x86-64 and ix86. Ok?
> >
> > * tree.c (build_common_tree_nodes): Don't even store the
> > __int128 types if they're not supported.
>
> OK.
Thanks! Committed.
> Please review the patch and let me know if there should be any modifications
> in it?
Have you checked the other alignment macros to see if they need to be
fixed too?
* config/rx/rx.h (Corrected macro LABEL_ALIGN).
This should be :
* config/rx/rx.h (LABEL_ALIGN):
It's alway
Minor tweak to space optimization. Committed.
* config/rx/rx.c (rx_max_skip_for_label): Don't skip anything if -Os.
Index: config/rx/rx.c
===
--- config/rx/rx.c (revision 211479)
+++ config/rx/rx.c (working copy)
The RX100/200 families have 4 byte cache lines, vs 8 bytes for the
RX600 family. Applied.
* config/rx/rx.h (FUNCTION_BOUNDARY): Adjust for RX100/200 4-byte
cache lines.
* config/rx/rx.c (rx_option_override): Likewise.
(rx_align_for_label): Likewise.
Index: config
If the combined bitfields are exactly the size of the mode, the logic
for detecting range overflow is flawed - it calculates an ending
"position" that's the position of the first bit in the next field.
In the case of "short" for example, you get "16 > 15" without this
patch (comparing size to pos
> Looks ok to me, but can you add a testcase please?
I have a testcase, but if -flto the testcase doesn't include *any*
definition of the test function, just all the LTO data. Is this
normal?
> Also check if 4.9 is affected.
It is... same fix works, though.
> Since the test on __STRICT_ANSI__ is removed for all other uses, it would
> seem consistent to me to remove this one as well. Besides, you are already
> testing __GLIBCXX_USE_INT_N_0, which as far as I understand is protected
> by !flag_iso (with the exception of size_t).
Yup, I'll clean tha
> The changes to dwarf2asm.c, cppbuiltin.c, optabs.c, defaults.h, expr.c,
> expmed.c, tree-dfa.c, simplify-rtx.c, lto-object.c, loop-iv.c, varasm.c,
> the msp430 back end and some of the stor-layout.c changes don't look like
> they should depend on the rest of the patch. I think it would help
Part 1 of 4, split from the full patch. The purpose of this set of
changes is to remove assumptions in GCC about type sizes. Previous to
this patch, GCC assumed that all types were powers-of-two in size, and
used naive math accordingly.
Old:
POINTER_SIZE / BITS_PER_UNIT
TYPE_SIZ
Part 3 of 4, split from the full patch. Additional optimization
opportunity, since the MSP430 does a lot of conversions between HImode
and PSImode.
gcc/
* expr.c (convert_move): If the target has an explicit converter,
use it.
Index: gcc/expr.c
==
Part 4 of 4, split from the full patch. This is the MSP430-specific
use of the new intN framework to enable true 20-bit pointers. Since
I'm one of the MSP430 maintainers, this patch is being posted for
reference, not for approval.
gcc/config/msp430
* config/msp430/msp430-protos.h (msp4
> The RX toolchain has a similar patch where the option is "doubles"
> while the folder name is "double".
There's no reason for one toolchain to be consistent with another,
though.
> Would it be ok to revert this line from "64-bit-doubles" to "64-bit-double"
> as this?
> +MULTILIB_DIRNAMES =
> https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01860.html
Build parts are approved.
> No stor-layout.c listed here but...
I knew I'd miss at least one in the split-up...
> > Index: gcc/stor-layout.c
> > ===
> > --- gcc/stor-layout.c (revision 211858)
> > +++ gcc/stor-layout.c (working copy)
> > @@ -2123
Are you proposing we remove TYPE_SIZE completely?
> Yes; I think that makes sense, unless someone produces a clearer
> definition of what TYPE_SIZE means that isn't redundant.
Does TYPE_SIZE have a different meaning than TYPE_PRECISION
for non-integer types? Floats, vectors, complex?
> If you find a particular use of TYPE_SIZE is using a size that isn't
> correct for your type whose precision is not a multiple of
> BITS_PER_UNIT, then in my model the correct fix is to change that
> use of TYPE_SIZE rather than to change the value of TYPE_SIZE for
> that type - and such a chang
> Do you have modes whose size is not multiple of the unit?
Yes. That's exactly the problem I'm trying to solve here. I'm making
partial int modes have real corresponding types, and they can be any
bit size, with target PS*modes to match. The MSP430, for example, has
20-bit modes, 20-bit opera
> And the hardware really loads 20 bits and not 24 bits? If so, I
> think you might want to consider changing the unit to 4 bits instead
> of 8 bits. If no, the mode is padded and has 24-bit size so why is
> setting TYPE_PRECISION to 20 not sufficient to achieve what you
> want?
The hardware tr
> That's what'll need fixing then.
Can I change TYPE_SIZE to TYPE_SIZE_WITH_PADDING then? Because it's
not reflecting the type's size any more. Why do we have to round up a
type's size anyway? That's a pointless assumption *unless* you're
allocating memory space for it, and in that case, you w
> I'm very skeptical... In any case, having a type whose TYPE_SIZE is smaller
> than the size of its MODE is a lie which will bite you back at some point.
Except gcc now knows the size of partial int modes. In this case,
PSImode is 20 bits and TYPE_SIZE is 20 bits, so they match.
The code was
> OK to apply ?
Ok but..
> +case SECCAT_TBSS:
> + return default_select_section (decl, reloc, align);
> +
> +default:
> + gcc_unreachable ();
Would it be better to just "default:" everything to
default_select_section, instead of enumerating everything we know
about today?
> If the other c file only includes libiberty.h and does not include the
> libiberty/config.h and
In general, such "other c files" should have their own config.h that
does the same test and has its own HAVE_DECL_ASPRINTF.
That way, the config.h matches the compiler options being used, and
not th
Copied the way other tests get uint32_t. Ok?
* gcc.c-torture/execute/pr65369.c: Don't assume int is 32 bits.
Index: gcc.c-torture/execute/pr65369.c
===
--- gcc.c-torture/execute/pr65369.c (revision 223737)
+++ gcc.c-tor
Done. Thanks!
20040625-1 fails on targets with pointers bigger than WORD_SIZE (rl78,
msp430/-mlarge) because the base register is clobbered, partially
rebuilt with the new value, then used as a base for the second part of
the calculation. Ok?
* expmed.c (extract_bit_field_1): Avoid clobbering a
A few minor nits...
Your patch includes many whitespace changes, which makes reviewing
your patch more difficult. Please limit whitespace changes when
they're unrelated to the patch.
Assuming you've looked at the actual diffs for them, and see nothing
beyond changes related to your patch, it's
> > * expmed.c (extract_bit_field_1): Avoid clobbering a
> > yet-to-be-used base/index register.
> OK.
> jeff
Thanks! committed.
libiberty is not an API to gcc, it is a portability library. If GCC
is exporting a timer, GCC's headers should have the interface in it.
This is the same patch as 09/16. There is only one libiberty master,
controlled by gcc, it is not neccessary to submit separate patches for
each copy of it.
The convention is: Any libiberty patch approved by gcc maintainers is
auto-approved for the other repos.
> +/* Fill an lvalue with zero bits. */
> +#define CLEAR_VAR(S) \
> + do { memset (&(S), 0, sizeof (S)); } while (0)
Hmmm... I don't see the point in this. The macro is almost as long as
a bare memset() would be, and replaces a well-known function with
something unknown outside this project.
> FWIW I'm not in love with the name of the macro, but I find it useful.
> In the initial version of patches 10 and 12 (state purging within "gas"
> and "ld" subdirs) I didn't have this macro, and had about 30 memset
> invocations.
Another option is to save the state as it was initialized, and re
This patch updates gthr-tpf.h to the current gthr.h API and TPF API. Ok?
* gthr-tpf.h (__GTHREADS_CXX0X): Define.
(__gthread_t): Define.
(__gthread_cond_t): Define.
(__gthread_time_t): Define.
(__GTHREAD_HAS_COND): Define.
(__GTHREAD_COND_INIT): De
Ok. Thanks!
101 - 200 of 581 matches
Mail list logo
