[Bug rtl-optimization/91865] Combine misses opportunity to remove (sign_extend (zero_extend)) before searching for insn patterns

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91865

--- Comment #5 from Jozef Lawrynowicz  ---
(In reply to Segher Boessenkool from comment #4)
> Created attachment 47550 [details]
> simplify-rtx patch for extends of AND of hard reg
> 
> So I did a patch to make this work (in simplify-rtx) also for hard registers
> (see attachment).

Confirmed that I can remove the unnamed RTL insns labelled under "BZ 91865" in
msp430.md and optimal code is generated for the first testcase when the patch
is applied.

[Bug target/92287] Mismatches in the calling convention for zero sized types

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92287

--- Comment #10 from Jozef Lawrynowicz  ---
(In reply to gnzlbg from comment #9)
> 
> @josef
> 
> > The MSP430 ABI is here: http://www.ti.com/lit/an/slaa534/slaa534.pdf
> Although confusingly that document is wrong regarding passing structures and
> unions by reference. As I mentioned before, structures and unions are always
> passed by reference, regardless of size.
> 
> Can you expand on this? That document says that aggregates smaller than
> 32-bit are passed in registers. We were trying to update our code
> documentation to cite the ABI specs and realized this. Do you have a link to
> where the current behavior is specified?

I think the ABI used to be correct regarding this, but then an optimization was
added to the TI compiler to always passes structures/unions by registers. At
least this is what I gleaned from searching the TI forums.

In the past TI also confirmed to me directly that that structs/unions should
always be passed by reference. I'll see if I can get them to update the ABI.

If you are curious about what any back-end is trying to do regarding
passing/returning by reference you could always check the implementation of
TARGET_PASS_BY_REFERENCE or TARGET_RETURN_IN_MEMORY.

[Bug target/70320] msp430 asm volatile does not accept lower-case register names in clobber list

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70320

Jozef Lawrynowicz  changed:

   What|Removed |Added

 CC||jozefl.gcc at gmail dot com

--- Comment #2 from Jozef Lawrynowicz  ---
This bug is fixed on trunk, I would appreciate if someone would close it for
me.

Thanks.

[Bug target/92287] Mismatches in the calling convention for zero sized types

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92287

--- Comment #3 from Jozef Lawrynowicz  ---
(In reply to gnzlbg from comment #2)
> > I can only speak for msp430, but there's no problem with that generated 
> > assembly. Structures and unions are always passed by reference.
> 
> I suppose that by this you mean that the current behavior is "by design", is
> that correct ?
> 
> If so, could you explain the rationale of this design or point me to the ABI
> specification document or rationale for it ?

I was just considering from an MSP430 point of view, that if the struct can
have an address (it looks like it can, even though it has zero size), then that
assembly is correct. I'm afraid I don't have any specific insight into how GCC 
generically handles zero sized structs beyond that though.

The MSP430 ABI is here: http://www.ti.com/lit/an/slaa534/slaa534.pdf
Although confusingly that document is wrong regarding passing structures and
unions by reference. As I mentioned before, structures and unions are always
passed by reference, regardless of size.

[Bug target/92287] Mismatches in the calling convention for zero sized types

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92287

Jozef Lawrynowicz  changed:

   What|Removed |Added

 CC||jozefl.gcc at gmail dot com

--- Comment #1 from Jozef Lawrynowicz  ---
I can only speak for msp430, but there's no problem with that generated
assembly. Structures and unions are always passed by reference.

R12:R15 are the argument registers, and the return value starts in R12.

[Bug rtl-optimization/91865] Combine misses opportunity to remove (sign_extend (zero_extend)) before searching for insn patterns

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91865

--- Comment #3 from Jozef Lawrynowicz  ---
Created attachment 46936
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46936=edit
msp430-extendhipsi2.diff

[Bug rtl-optimization/91865] Combine misses opportunity to remove (sign_extend (zero_extend)) before searching for insn patterns

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91865

--- Comment #2 from Jozef Lawrynowicz  ---
A related issue can be observed if "char i" is made global instead of an
argument to func.

const int table[2] = {1, 2};

int foo;
char i;

void func(void)
{
  foo = table[i];
}

Combine combines the zero_extend and sign_extend insns into a single
(sign_extend(subreg)) insn:

Trying 6 -> 7:
6: r27:HI=zero_extend([`i'])
7: r28:PSI=sign_extend(r27:HI)#0
  REG_DEAD r27:HI
Failed to match this instruction:
(set (reg:PSI 28 [ i ])
(sign_extend:PSI (subreg:HI (mem/c:QI (symbol_ref:PSI ("i")

As far as I'm aware, this is a "discouraged" use of subreg, but also means we
miss out on the best match.

Note that the subreg in the combined instruction is unrelated to the subreg
that is the rvalue of insn 7. The subreg in insn 7 is from the RTL pattern for
extendhipsi2.
To avoid any confusion I've attached a one line patch
(msp430-extendhispsi2.diff) which removes the the subreg from this RTL insn
pattern, yet the the subreg in the insn combine searches for remains. Here is
the output from combine with the patch (note the the lack of subreg in insn 7):

Trying 6 -> 7:  
6: r27:HI=zero_extend([`i'])
7: r28:PSI=sign_extend(r27:HI)  
  REG_DEAD r27:HI   
Successfully matched this instruction:  
(set (reg:PSI 28 [ i ]) 
(sign_extend:PSI (subreg:HI (mem/c:QI (symbol_ref:PSI ("i")
allowing combination of insns 6 and 7   
original costs 16 + 8 = 24  
replacement cost 20 
deferring deletion of insn with uid = 6.
modifying insn i3 7: r28:PSI=sign_extend([`i']#0)   
deferring rescan insn with uid = 7.

Even though we matched, it is undesirable since we matched the costly
"extendhipsi2".
We want to match movqipsi2, which would happen if:
(sign_extend:PSI (zero_extend:HI (mem:QI))) -> (zero_extend:PSI (mem:QI))

P.S. I believe the subreg in some of the msp430 RTL patterns is because of
historical issues with reload and/or optimization tweaks. The attached patch is
just to clarify the behaviour I mentioned and not necessarily beneficial
overall.

[Bug rtl-optimization/91865] New: Combine misses opportunity to remove (sign_extend (zero_extend)) before searching for insn patterns

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91865

Bug ID: 91865
   Summary: Combine misses opportunity to remove (sign_extend
(zero_extend)) before searching for insn patterns
   Product: gcc
   Version: 9.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: rtl-optimization
  Assignee: unassigned at gcc dot gnu.org
  Reporter: jozefl.gcc at gmail dot com
  Target Milestone: ---

Created attachment 46913
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46913=edit
msp430-movqipsi.diff

The following program generates poor code for msp430-elf in the large memory
model:

const int table[2] = {1, 2};

int
foo (char i)
{
  return table[i];
}

> msp430-elf-gcc -S tester.i -mlarge -Os
The RTL generated by expand uses two insns to convert "i" to a register's
natural mode; there is a sign extension which would be unnecessary if the first
instruction had a PSImode register as the lvalue:

(insn 2 4 3 2 (set (reg/v:HI 25 [ i ])
(zero_extend:HI (reg:QI 12 R12 [ i ])))
 (nil))
.
(insn 7 6 8 2 (set (reg:PSI 28)
(subreg:PSI (sign_extend:SI (reg/v:HI 25 [ i ])) 0))
 (nil))

All we really need is:

(insn (set (reg:PSI 28 [ i ])
(zero_extend:PSI (reg:QI 12 R12 [ i ])))
 (nil))

Combine tries to combine the insns but doesn't recognize that 
the following transformation could be applied:
(sign_extend:PSI (zero_extend:HI)) -> (zero_extend:PSI)

Trying 2 -> 7:
2: r25:HI=zero_extend(R12:QI)
  REG_DEAD R12:QI
7: r28:PSI=sign_extend(r25:HI)#0
  REG_DEAD r25:HI
Failed to match this instruction:
(set (reg:PSI 28 [ i ])
(sign_extend:PSI (zero_extend:HI (reg:QI 12 R12 [ i ]
Failed to match this instruction:
(set (reg:PSI 28 [ i ])
(sign_extend:PSI (and:HI (reg:HI 12 R12)
(const_int 255 [0xff]


A separate issue is that the movqipsi pattern is undefined, a patch containing
the pattern is attached and would match if the above transformation were
applied by combine.

[Bug target/90175] ambiguous wording "critical attribute" in diagnostic

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90175

Jozef Lawrynowicz  changed:

   What|Removed |Added

 CC||jozefl.gcc at gmail dot com

--- Comment #1 from Jozef Lawrynowicz  ---
(In reply to Roland Illig from comment #0)
> While here, I noticed that the warning message "naked functions cannot be
> reentrant" is never translated properly. This message, and the other ones
> must be enclosed in N_(...).
What is the difference between G_(...) and N_(...)?

I could only find current documentation regarding G_(...) in gcc/ABOUT-GCC-NLS:

> The `G_(GMSGID)' macro defined in intl.h can be used to mark GCC diagnostics
> format strings as requiring translation, but other than that it is a
> no-op at runtime.
r42965 back in 2001 added documentation about N_(...) but it has subsequently
been removed:

> Non-empty description strings should be marked with @code{N_(@dots{})} for
> @command{xgettext}.  In addition to the description for @option{--help},
> more detailed documentation for each option should be added to
> @file{invoke.texi}.
Both G_(...) and N_(...) appear sparingly used in the any of the back ends.
Most back ends don't use them at all.

As far as I understand, strings which get passed to warning() or error() or
other functions with arguments ending in gmsgid don't need to be wrapped with
G_() or N_().  From gcc/ABOUT-GCC-NLS again:

> If the parameter name ends with `gmsgid', it is assumed to be a GCC
> diagnostics format string requiring translation, if it ends with
> `cmsgid', it is assumed to be a format string for `printf' family
> of functions, requiring a translation.
So I guess in the case of this message about the "critical" attribute, it
should be reworded and critical wrapped in %< %> but I don't think it needs the
N_() or G_().

The instance when we do have wrapping of messages in G_(...) in msp430.c is
when the string is stored in a variable which is later passed to warning() (see
msp430_attr() for example). I'm guessing this is necessary because the
translation requirement somehow isn't set on the string if it is stored in a
variable before being passed to the function.

[Bug target/91306] [MSP430] libgcc/crtstuff.c: Alignment of frame_dummy .init_array entry is too big

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91306

Jozef Lawrynowicz  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |FIXED

--- Comment #6 from Jozef Lawrynowicz  ---
Fixed on trunk.

[Bug target/91306] [MSP430] libgcc/crtstuff.c: Alignment of frame_dummy .init_array entry is too big

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91306

--- Comment #4 from Jozef Lawrynowicz  ---
Should I submit a patch which changes uses of sizeof in alignment attributes to
__alignof__? Or are you working on it?

[Bug target/91306] [MSP430] libgcc/crtstuff.c: Alignment of frame_dummy .init_array entry is too big

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91306

--- Comment #3 from Jozef Lawrynowicz  ---
(In reply to Andrew Pinski from comment #2)
> If that is true, then maybe alignof should be used instead of sizeof.
> Can you try alignof (__alignof__)?

Thanks, __alignof__ works for me. Below change fixed the issue for
msp430-elf/-mlarge:

> diff --git a/libgcc/crtstuff.c b/libgcc/crtstuff.c
> index 4927a9f8977..bd5c93c194e 100644
> --- a/libgcc/crtstuff.c
> +++ b/libgcc/crtstuff.c
> @@ -474,7 +474,7 @@ frame_dummy (void)
>  CRT_CALL_STATIC_FUNCTION (__LIBGCC_INIT_SECTION_ASM_OP__, frame_dummy)
>  #else /* defined(__LIBGCC_INIT_SECTION_ASM_OP__) */
>  static func_ptr __frame_dummy_init_array_entry[]
> -  __attribute__ ((__used__, section(".init_array"), 
> aligned(sizeof(func_ptr
> +  __attribute__ ((__used__, section(".init_array"), 
> aligned(__alignof__(func_ptr
>= { frame_dummy };
>  #endif /* !defined(__LIBGCC_INIT_SECTION_ASM_OP__) */
>  #endif /* USE_EH_FRAME_REGISTRY || USE_TM_CLONE_REGISTRY */

The other structures using "aligned(sizeof(func_ptr))" are unused for msp430,
so they aren't causing problems, but alignof does seem like a better keyword to
use for those if it works for the other targets as well.

[Bug libgcc/91306] New: [MSP430] libgcc/crtstuff.c: Alignment of frame_dummy .init_array entry is too big

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91306

Bug ID: 91306
   Summary: [MSP430] libgcc/crtstuff.c: Alignment of frame_dummy
.init_array entry is too big
   Product: gcc
   Version: 9.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: libgcc
  Assignee: unassigned at gcc dot gnu.org
  Reporter: jozefl.gcc at gmail dot com
  Target Milestone: ---

In libgcc/crtstuff.c, an alignment of "sizeof (void *)" (i.e. pointer size)
is enforced for the frame_dummy() init_array entry. For msp430-elf in the
large memory model (-mlarge), pointer size is 4 bytes, but the expected
alignment of a pointer is 2 bytes.

So when .init_array is constructed, there could be padding added at the
beginning or between entries, since the structure itself is only aligned to
2 bytes but this entry is aligned to 4 bytes.
The padding causes incorrect execution since the code to run through
.init_array does not expect any gaps between entries since pointers only need
to be 2 byte aligned.

I see this alignment of "sizeof (void *)" was added to fix a mips64 bootstrap
failure in r182066 (https://gcc.gnu.org/ml/gcc-patches/2011-12/msg00393.html),
but this alignment isn't appropriate for msp430.

In crtstuff.c, the type of the init_array entry for frame_dummy is specified 
to be "void *", so why must the alignment also be specified?
I don't have access to a mips platform, but shouldn't the entry have the
correct alignment for the type?

Was the addition of the aligned attribute in the 2011 commit a workaround for a
GCC bug? Can it be removed now?

For the following code, GCC generates the the correct alignment without using
the "aligned" attribute on x86, x86_64 and msp430-elf/-mlarge.

static void * entry[]
__attribute__ ((used, section(".init_array"))) = { foo };

x86_64-pc-linux-gnu -m32:
>   .section.init_array,"aw"
>   .align 4
>   .type   entry, @object
>   .size   entry, 4
>entry:
>   .long   foo

x86_64-pc-linux-gnu -m64:
>   .section.init_array,"aw"
>   .align 8
>   .type   entry, @object
>   .size   entry, 8
>entry:
>   .quad   foo

msp430-elf -mlarge:
>   .section.init_array,"aw"
>   .balign 2
>   .type   entry, @object
>   .size   entry, 4
>entry:
>   .long   foo

[Bug testsuite/90812] Tests misuse "dg-require-effective-target int32plus" to check for 64-bit integer support

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90812

--- Comment #3 from Jozef Lawrynowicz  ---
(In reply to Richard Biener from comment #2)
> I think most tests like this end up using 'long long' and use
> __SIZEOF_LONG_LONG__ to guard code.  There's a dejagnu effective target for
> long long support.

The problem is that the effective target keyword that checks that long long is
64-bits, also checks that int and long are 32-bits:

> llp64 Target has 32-bit int and long, 64-bit long long and pointers.

msp430-elf has 16-bit int, 32-bit long and 64-bit long long.

And whilst almost all targets have 64-bit long long, avr can have 32-bit long
long in one configuration.

So I think an effective target to solely check that __INT64_TYPE__ is supported
would be useful.
I prefer this to guarding the code with sizeof(long long), as with
"dg-require-effective target int64type", the testsuite will recognize the test
is unsupported, rather than doing nothing and passing.

[Bug testsuite/90812] Tests misuse "dg-require-effective-target int32plus" to check for 64-bit integer support

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90812

--- Comment #1 from Jozef Lawrynowicz  ---
To check for 64-bit integer support, it seems that the "stdint_types" effective
target keyword should be sufficient, although I'm not sure if it might be
possible for a target to provide __INT64_TYPE__ but not stdint.h.

Perhaps a new effective target keyword "int64type", which runs the following,
is required:
#ifndef __INT64_TYPE__
#error
#endif

[Bug testsuite/90812] New: Tests misuse "dg-require-effective-target int32plus" to check for 64-bit integer support

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90812

Bug ID: 90812
   Summary: Tests misuse "dg-require-effective-target int32plus"
to check for 64-bit integer support
   Product: gcc
   Version: 9.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: testsuite
  Assignee: unassigned at gcc dot gnu.org
  Reporter: jozefl.gcc at gmail dot com
  Target Milestone: ---

Some tests which require 64-bit integer support via __INT64_TYPE__ use the
int32plus effective target keyword, apparently to try to check that this type
is supported.

However, int32plus only checks that "sizeof (int) >= 4", whilst some targets
have "sizeof(int) < 4", but still support __INT64_TYPE__.

The result of this is that some of these tests for 64-bit int behavior are not
run on 8-bit/16-bit int targets.

I identified at least gcc.dg/pr80131-1.c and gcc.dg/torture/pr86554-2.c to have
this problem. If I replace the "int32plus" requirement with "stdint_types",
then these tests pass on msp430-elf, which has "sizeof(int) == 2".

[Bug rtl-optimization/90032] [MSP430] reload uses wrong stack slot for variable after setjmp/longjmp

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90032

--- Comment #4 from Jozef Lawrynowicz  ---
Created attachment 46122
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46122=edit
tester.s

[Bug rtl-optimization/90032] [MSP430] reload uses wrong stack slot for variable after setjmp/longjmp

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90032

--- Comment #3 from Jozef Lawrynowicz  ---
Created attachment 46121
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46121=edit
tester.i reload dump

[Bug rtl-optimization/90032] [MSP430] reload uses wrong stack slot for variable after setjmp/longjmp

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90032

--- Comment #2 from Jozef Lawrynowicz  ---
Created attachment 46120
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46120=edit
tester.i ira dump

[Bug rtl-optimization/90032] [MSP430] reload uses wrong stack slot for variable after setjmp/longjmp

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90032

Jozef Lawrynowicz  changed:

   What|Removed |Added

  Attachment #46118|0   |1
is obsolete||

--- Comment #1 from Jozef Lawrynowicz  ---
Created attachment 46119
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46119=edit
tester.i

[Bug rtl-optimization/90032] New: [MSP430] reload uses wrong stack slot for variable after setjmp/longjmp

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90032

Bug ID: 90032
   Summary: [MSP430] reload uses wrong stack slot for variable
after setjmp/longjmp
   Product: gcc
   Version: 9.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: rtl-optimization
  Assignee: unassigned at gcc dot gnu.org
  Reporter: jozefl.gcc at gmail dot com
  Target Milestone: ---

Created attachment 46118
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46118=edit
tester.i

gcc.dg/torture/stackalign/setjmp-1.c fails at execution for msp430-elf at -O1.
An RTL optimization added in r255136 exposed the failure, but if I make this
optimization in the source of the test, then versions of GCC before this also
fail, back to GCC 7. It passes with gcc-6.4, but again that is just because the
RTL at reload is different.

I've attached a reduced testcase (tester.i) based on setjmp-1.c.
> msp430-elf-gcc -O1 -msim tester.i
The failure occurs because the wrong stack slot is used as the first argument
to strcmp.
R1 is the stack pointer, R12 stores the first argument to functions.
>   MOV.W   R12, 20(R1); The address of the string is stored in 20(R1)
>   MOV.W   #25972, @R12   ; "te"
>   MOV.W   #29811, 2(R12) ; "st"
> (No other modifications of R1)
> preparing jmp_buf
>   CALL#sub2
> Label for return from longjmp
>   MOV.W   @R1, R12  ; The address of the string is actually in 20(R1)
>   CALL#strcmp
Whether the test fails or not seems gated on if frame_pointer_needed == true
for main(). When there is "more" RTL code (i.e. before the revisions that added
the problematic optimizations), then frame_pointer_needed == true so the
address of the "test" string will be used as an offset from the frame pointer,
instead of the stack pointer.

TARGET_FRAME_POINTER_REQUIRED () always returns false for msp430, but if I make
it return true if
  (cfun->has_nonlocal_label || cfun->calls_setjmp)
then the test passes and the following code is generated.

The frame pointer is R4, but it is not fixed.
>   MOV.W   R12, -2(R4)
>   MOV.W   #25972, @R12
>   MOV.W   #29811, 2(R12)
>  (No other modifications of R4)
>   CALL#sub2
>  Label for return from longjmp
>   MOV.W   -2(R4), R12  ; -2(R4) contains the correct address of "test"

I've attached the assembly file, IRA and reload dumps for tester.i when
compiled with current trunk.

[Bug target/84406] [8 Regression][MSP430] ICE on valid code in find_widening_optab_handler_and_mode, at optabs-query.c:476

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84406

--- Comment #12 from Jozef Lawrynowicz  ---
(In reply to rsand...@gcc.gnu.org from comment #11)
> Created attachment 43437 [details]
> Possible patch v2
> 
> This time for real.

Thanks, patch looks good to me, and the original testcase does indeed pass with
it applied.

Trunk still doesn't build for msp430-elf, but that seems to be an unrelated bug
in libstdc++.

[Bug target/84406] [8 Regression][MSP430] ICE on valid code in find_widening_optab_handler_and_mode, at optabs-query.c:476

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84406

--- Comment #3 from Jozef Lawrynowicz  ---
Created attachment 43433
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43433=edit
define {u,}mulhipsi3 insns

I applied the patch but trunk still ICEs.

After defining some insns for "umulhipsi3" and "mulhipsi3" in msp430.md, the
build gets a little further, but then a case is encountered where a widen from
QI to PSI is attempted. QI != PARTIAL_INT_MODE, so we ICE again.

The insns in attached patch are mostly just copied from the mulhisi*, so are
almost definitely not correct, but they are at least there for GCC to use in
the build.

[Bug target/79242] [7 Regression] ICE in simplify_subreg, at simplify-rtx.c:6029

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79242

--- Comment #11 from Jozef Lawrynowicz  ---
(In reply to Eric Gallager from comment #10)
> (In reply to Jeffrey A. Law from comment #9)
> > Author: law
> > Date: Wed Feb 14 07:21:11 2018
> > New Revision: 257653
> > 
> > URL: https://gcc.gnu.org/viewcvs?rev=257653=gcc=rev
> > Log:
> > 2018-02-14  Jozef Lawrynowicz 
> > 
> > PR target/79242
> > * machmode.def: Define a complex mode for PARTIAL_INT.
> > * genmodes.c (complex_class): Return MODE_COMPLEX_INT for
> > MODE_PARTIAL_INT.
> > * doc/rtl.texi: Document CSPImode.
> > * config/msp430/msp430.c (msp430_hard_regno_nregs): Add CPSImode
> > handling.
> > (msp430_hard_regno_nregs_with_padding): Likewise.
> > 
> > PR target/79242
> > * gcc.target/msp430/pr79242.c: New test.
> > 
> > Added:
> > trunk/gcc/testsuite/gcc.target/msp430/pr79242.c
> > Modified:
> > trunk/gcc/ChangeLog
> > trunk/gcc/config/msp430/msp430.c
> > trunk/gcc/doc/rtl.texi
> > trunk/gcc/genmodes.c
> > trunk/gcc/machmode.def
> > trunk/gcc/testsuite/ChangeLog
> 
> Did this fix it?

The patch is currently only on trunk, but after back-porting it to
gcc-7-branch, GCC now builds successfully for the msp430-elf target.
The original testcase also compiles successfully with the resulting toolchain.

Trunk still doesn't build for msp430-elf though, due to Bug 84406.

[Bug target/84406] New: [8 Regression][MSP430] ICE on valid code in find_widening_optab_handler_and_mode, at optabs-query.c:476

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84406

Bug ID: 84406
   Summary: [8 Regression][MSP430] ICE on valid code in
find_widening_optab_handler_and_mode, at
optabs-query.c:476
   Product: gcc
   Version: 8.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: target
  Assignee: unassigned at gcc dot gnu.org
  Reporter: jozefl.gcc at gmail dot com
  Target Milestone: ---

Trunk currently fails to build for the msp430-elf target due to an ICE. 

A reduced testcase is below.

# 1 "testcase.c"
# 1 ""
# 1 ""
# 1 "testcase.c"
void a (void) {
  __int20 a, b;
  __int20 c = a * b;
}

$ ./gcc/xgcc -B./gcc/ -S testcase.i

during RTL pass: expand
testcase.c: In function 'a':
testcase.c:3:11: internal compiler error: in
find_widening_optab_handler_and_mode, at optabs-query.c:476
   __int20 c = a * b;
   ^
0xaaa5cc find_widening_optab_handler_and_mode(optab_tag, machine_mode,
machine_mode, machine_mode*)
../../gcc/optabs-query.c:476
0xa9f399 expand_binop(machine_mode, optab_tag, rtx_def*, rtx_def*, rtx_def*,
int, optab_methods)
../../gcc/optabs.c:1290
0x8355fd expand_mult(machine_mode, rtx_def*, rtx_def*, rtx_def*, int, bool)
../../gcc/expmed.c:3536
0x85b492 expand_expr_real_2(separate_ops*, rtx_def*, machine_mode,
expand_modifier)
../../gcc/expr.c:8946
0x726b49 expand_gimple_stmt_1
../../gcc/cfgexpand.c:3730
0x726b49 expand_gimple_stmt
../../gcc/cfgexpand.c:3790
0x72881b expand_gimple_basic_block
../../gcc/cfgexpand.c:5819
0x72e196 execute
../../gcc/cfgexpand.c:6425

Started with:

commit d2a1b4530f1d00fb35c2aee051b00398a624bd27
Author: rsandifo <rsandifo@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Nov 1 12:30:39 2017 +

Widening optab cleanup
...

The ICE appears to be caused by the fact there is no widening multiply insn for
PSImode currently implemented for msp430, which is what
find_widening_optab_handler_and_mode is searching for.

[Bug target/79242] ICE in simplify_subreg, at simplify-rtx.c:6029

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79242

--- Comment #8 from Jozef Lawrynowicz  ---
Proposed patch: https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01108.html

[Bug target/79242] ICE in simplify_subreg, at simplify-rtx.c:6029

[jozefl.gcc at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79242

Jozef Lawrynowicz  changed:

   What|Removed |Added

 CC||jozefl.gcc at gmail dot com

--- Comment #7 from Jozef Lawrynowicz  ---
(In reply to Andrei Pushkin from comment #6)
> Still broken in 7.2
> 
> What are the chances of this bug getting some love?

I'm happy to report that I am back working on MSP430 GCC. However, I have some
other startup items to complete before I can get back to fixing bugs, but this
is top of my bug list.