> -----Original Message-----
> From: Michael Eager [mailto:ea...@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.holsgr...@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.holsgr...@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.holsgr...@xilinx.com>
   * config/microblaze/microblaze.md (cstoresf4, cbranchsf4): Replace
      comparison_operator with ordered_comparison_operator.

thanks again,
David

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



Reply via email to