[PATCH] D111085: [FPEnv][InstSimplify] Fold constrained X + -0.0 ==> X
Serge Pavlov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 6 10:16:16 PDT 2021
sepavloff added inline comments.
================
Comment at: llvm/include/llvm/IR/FPEnv.h:64
+/// at run time.
+inline bool RoundingModeMayBe(RoundingMode RM, RoundingMode QRM) {
+ return RM == QRM || RM == RoundingMode::Dynamic;
----------------
kpn wrote:
> spatel wrote:
> > kpn wrote:
> > > spatel wrote:
> > > > Nit: If we don't use the current/correct formatting for a function name (`roundingModeMayBe`), we're never going to get out of the formatting mess we're in.
> > > >
> > > > Bonus points for fixing all of the functions here (as an NFC preliminary patch). :)
> > > Does this apply to type names that appear in the name of functions? For example, RoundingModeToStr() takes a RoundingMode argument. That was part of why I went with RoundingModeMayBe(). (The rest of the reason was to be consistent with the rest of this file.)
> > I didn't notice that quirk of naming...
> > In this case, I think we can fix it with "canRoundingModeBe" ?
> > That may lower our score on an English test, but still in compliance with:
> > https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> >
> >
> That works!
>
> Oh, @sepavloff, are you good with me adding, in a different commit, the word "convert" to the beginning of these four conversion functions?
I am OK with it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111085/new/
https://reviews.llvm.org/D111085
More information about the llvm-commits
mailing list