[PATCH] Add support for generating MIPS legacy NaN
Daniel Sanders
daniel.sanders at imgtec.com
Thu Feb 26 08:31:52 PST 2015
================
Comment at: lib/Basic/Targets.cpp:5679
@@ +5678,3 @@
+ bool isNan2008() const override {
+ return IsNan2008 || isNaN2008Default();
+ }
----------------
vradosavljevic wrote:
> vradosavljevic wrote:
> > petarj wrote:
> > > dsanders wrote:
> > > > petarj wrote:
> > > > > vradosavljevic wrote:
> > > > > > dsanders wrote:
> > > > > > > This isn't quite right. It should just be IsNan2008 since IsNan2008 is initialized with the result of isNaN2008Default().
> > > > > > When you generating code for mips32r6/mips64r6 with option -mnan=legacy, IsNan2008 will be set to false.
> > > > > > In function handleTargetFeatures there are lines that change that.
> > > > > > else if (*it == "-nan2008")
> > > > > > IsNan2008 = false;
> > > > > > Because of that, I use the result of isNaN2008Default().
> > > > > We should not allow that case. Clang should (at least) issue a warning if "-mnan=legacy" is passed for the revisions that do not support the NaN legacy encoding.
> > > > Agreed. I'd prefer an error* since the user is requesting an impossible combination of options but it should at least warn instead of quietly ignoring the user.
> > > >
> > > > *Ideally with a suggestion for fixing it, e.g. "error: IEEE754(2008) compliant NaN's are mandatory on Mips32r6. Did you mean -mnan=2008?"
> > > I would also prefer an error, but gcc seems to produce a warning only.
> > > Furthermore, we should do similar for "-mnan=2008" and earlier mips revisions. But this all can be part of another patch, not this one necessary.
> > Done.
> Ok. This will be part of the next patch.
> Furthermore, we should do similar for "-mnan=2008" and earlier mips revisions. But this all can be part of another patch, not this one necessary.
Off hand, I'm not sure exactly which ISA introduced the ABS2008 and NAN2008 config bits but at least some of the ISA's prior to 32r6/64r6 supported 2008 in addition to legacy. I don't think it's seen widespread use though.
http://reviews.llvm.org/D7882
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list