[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