[Patch, microblaze]: Fix ICE with mhard-float

[David Holsgrove]

> -----Original Message-----
> From: Michael Eager [mailto:eager@eagerm.com]
> Sent: Friday, 17 January 2014 4:36 am
> To: David Holsgrove; gcc-patches@gcc.gnu.org
> Cc: Edgar Iglesias; John Williams; Vidhumouli Hunsigida; Nagaraju Mekala
> Subject: Re: [Patch, microblaze]: Fix ICE with mhard-float
> 
> On 11/25/13 23:50, David Holsgrove wrote:
> > Hi Michael,
> >
> > I've attached the same patch based on latest gcc master.
> >
> > Can this be applied to gcc-4_8-branch also.
> >
> > thanks,
> > David
> >
> > On 15 July 2013 14:53, David Holsgrove <david.holsgrove@xilinx.com> wrote:
> >> Add SImode to cstoresf4's comparison operator, prevents ICE during combine
> >> rtl pass with error message;
> >>
> >> internal compiler error: in simplify_subreg, at simplify-rtx.c:5725
> >>
> >> Use ordered_comparison_operator predicate to limit operators to
> >> those fcmp can handle, and letting compiler reorder insns to
> >> accommodate unordered as necessary.
> >>
> >> Changelog entry;
> >>
> >> 2013-07-15  David Holsgrove <david.holsgrove@xilinx.com>
> >>
> >>   * gcc/config/microblaze/microblaze.md: Fix cstoresf4 and cbranchsf4
> >>
> >> Can this be backported to gcc-4_8-branch also?
> 
> 
> Hi David --
> 
> You mention that this patch fixes an ICE.  Is there a failing test
> case in the GCC Test Suite?  Is there a GCC PR for this ICE?
> If not, please add a test case to the patch.

Hi Michael,

Thanks for the reply. No, unfortunately there isn’t an existing test case which shows this ICE.

I'm in the process of trying to recreate and distil into a small test case if possible, but the error was encountered whilst building a rather large app using microblaze linux.

From the documentation though we can see that the use of comparison_operator as the predicate for these Microblaze insn patterns is too broad, as fcmp only accepts the following;

http://www.xilinx.com/support/documentation/sw_manuals/xilinx14_7/mb_ref_guide.pdf 
fcmp.un rD, rA, rB Unordered floating point comparison
fcmp.lt rD, rA, rB Less-than floating point comparison
fcmp.eq rD, rA, rB Equal floating point comparison
fcmp.le rD, rA, rB Less-or-Equal floating point comparison
fcmp.gt rD, rA, rB Greater-than floating point comparison
fcmp.ne rD, rA, rB Not-Equal floating point comparison
fcmp.ge rD, rA, rB Greater-or-Equal floating point comparison

From gcc/gensupport.c we see that the comparison_operator predicate consists of the following expression codes;

EQ, NE, LE, LT, GE, GT, LEU, LTU, GEU, GTU, UNORDERED, ORDERED, UNEQ, UNGE, UNGT, UNLE, UNLT, LTGT

Whereas ordered_comparison_operator has;
EQ, NE, LE, LT, GE, GT, LEU, LTU, GEU, GTU

(microblaze.c's print_operand handles the unordered comparison modes before passing to fcmp)

The GCC Internals documentation also highlights that ordered_comparison_operator should likely be used to restrict the possible comparison modes;

http://gcc.gnu.org/onlinedocs/gccint/Standard-Names.html
‘cstoremode4’
These operations may FAIL, but should do so only in relatively uncommon cases; if they would FAIL for common cases involving integer comparisons, it is best to restrict the predicates to not allow these operands. Likewise if a given comparison operator will always fail, independent of the operands (for floating-point modes, the ordered_comparison_operator predicate is often useful in this case).

> 
> Changelog entries saying "fix XYZ" are not as useful as a description
> of the changes made.  A better Changelog would be:
> 
>     * config/microblaze/microblaze.md (cstoresf4, cbranchsf4): Replace
>       comparison_operator with ordered_comparison_operator.
>

Thanks for the suggestion, yes indeed, a better ChangeLog entry would be;

2013-07-15  David Holsgrove <david.holsgrove@xilinx.com>
   * config/microblaze/microblaze.md (cstoresf4, cbranchsf4): Replace
      comparison_operator with ordered_comparison_operator.

thanks again,
David

> 
> --
> Michael Eager	 eager@eagercon.com
> 1960 Park Blvd., Palo Alto, CA 94306  650-325-8077