[PATCH] D77247: Merge FloatingPointMode and FPEnv headers

Serge Pavlov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 2 08:39:14 PDT 2020


sepavloff added a comment.

In D77247#1957276 <https://reviews.llvm.org/D77247#1957276>, @arsenm wrote:

> In D77247#1957027 <https://reviews.llvm.org/D77247#1957027>, @arsenm wrote:
>
> > In D77247#1955942 <https://reviews.llvm.org/D77247#1955942>, @cameron.mcinally wrote:
> >
> > > The `fp::` qualifier is kind of verbose. I'm not familiar with LLVM's C++ rules, but can we replace them with `using namespace fp;`? Or can we remove the `fp` namespace completely?
> >
> >
> > I’m not sure, I just merged into FPEnv and followed what it was doing.
>
>
> I think llvm tends to use namespaces around enums as a substitute to have qualified enum values (which I guess is a poor substitute for enum class). I would be fine eliminating the namespace and switching the existing enums to use enum class.


Using C++ strict enums looks a better way than namespaces.

Interestingly, I am working on similar patch, that unifies rounding mode enums. But I have to move the enum `RondingMode` to `ADT/FloatingPointMode.h` because the rounding mode enum is used in APFloat and it is not a good thing to refer IR header in Support library. If you consider using `DenormalMode` in APFloat (for example to implement constant folding dependent on this mode) you might consider keeping its definition in more common place.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77247/new/

https://reviews.llvm.org/D77247





More information about the llvm-commits mailing list