This revision was automatically updated to reflect the committed changes.
Closed by commit rG85d049a089d4: Implement support for option
fexcess-precision. (authored by zahiraam).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
zahiraam updated this revision to Diff 484882.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
Files:
clang/docs/UsersManual.rst
clang/include/clang/AST/Type.h
clang/include/clang/Basic/FPOptions.def
andrew.w.kaylor accepted this revision.
andrew.w.kaylor added a comment.
This revision is now accepted and ready to land.
With the latest test cases and explanations, I'm happy with this patch.
@rjmccall is there anything else you wanted to see addressed?
CHANGES SINCE LAST ACTION
rjmccall added inline comments.
Comment at: clang/test/CodeGen/X86/fexcess-precision.c:89
+// CHECK-EXT-NEXT:[[EXT1:%.*]] = fpext half [[TMP1]] to float
+// CHECK-EXT-NEXT:[[MUL:%.*]] = fmul float [[EXT]], [[EXT1]]
+// CHECK-EXT-NEXT:[[TMP2:%.*]] = load half, ptr
andrew.w.kaylor added inline comments.
Comment at: clang/test/CodeGen/X86/fexcess-precision.c:89
+// CHECK-EXT-NEXT:[[EXT1:%.*]] = fpext half [[TMP1]] to float
+// CHECK-EXT-NEXT:[[MUL:%.*]] = fmul float [[EXT]], [[EXT1]]
+// CHECK-EXT-NEXT:[[TMP2:%.*]] = load half,
zahiraam updated this revision to Diff 483167.
zahiraam marked an inline comment as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
Files:
clang/docs/UsersManual.rst
clang/include/clang/AST/Type.h
pengfei added inline comments.
Comment at: clang/test/CodeGen/X86/fexcess-precision.c:89
+// CHECK-EXT-NEXT:[[EXT1:%.*]] = fpext half [[TMP1]] to float
+// CHECK-EXT-NEXT:[[MUL:%.*]] = fmul float [[EXT]], [[EXT1]]
+// CHECK-EXT-NEXT:[[TMP2:%.*]] = load half, ptr
andrew.w.kaylor added inline comments.
Comment at: clang/test/CodeGen/X86/fexcess-precision.c:89
+// CHECK-EXT-NEXT:[[EXT1:%.*]] = fpext half [[TMP1]] to float
+// CHECK-EXT-NEXT:[[MUL:%.*]] = fmul float [[EXT]], [[EXT1]]
+// CHECK-EXT-NEXT:[[TMP2:%.*]] = load half,
zahiraam added a comment.
ping
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zahiraam updated this revision to Diff 480198.
zahiraam edited the summary of this revision.
Herald added subscribers: pcwang-thead, nlopes, abrachet, sstefan1, phosek,
s.egerton, ormris, mstorsjo, simoncook, fedor.sergeev, aheejin, dschuff.
Herald added a reviewer: jdoerfert.
Herald added a
aaron.ballman added a reviewer: MaskRay.
aaron.ballman added a comment.
Herald added a subscriber: StephenFan.
Adding @MaskRay for driver-specific expertise.
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:821
+(Precision ==
zahiraam added inline comments.
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:821
+(Precision == LangOptions::ExcessPrecisionKind::FPP_Standard ||
+ Precision == LangOptions::ExcessPrecisionKind::FPP_Fast)) {
if (Ty->isAnyComplexType()) {
zahiraam updated this revision to Diff 479264.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
Files:
clang/docs/UsersManual.rst
clang/include/clang/Basic/FPOptions.def
clang/include/clang/Basic/LangOptions.def
rjmccall added inline comments.
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:821
+(Precision == LangOptions::ExcessPrecisionKind::FPP_Standard ||
+ Precision == LangOptions::ExcessPrecisionKind::FPP_Fast)) {
if (Ty->isAnyComplexType()) {
zahiraam marked 2 inline comments as done.
zahiraam added inline comments.
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:821
+(Precision == LangOptions::ExcessPrecisionKind::FPP_Standard ||
+ Precision == LangOptions::ExcessPrecisionKind::FPP_Fast)) {
if
rjmccall added inline comments.
Comment at: clang/include/clang/Basic/LangOptions.h:301
+ enum Float16ExcessPrecisionKind { FPP_Standard, FPP_Fast, FPP_None };
+
zahiraam wrote:
> rjmccall wrote:
> > rjmccall wrote:
> > > You can leave this named
zahiraam updated this revision to Diff 478738.
zahiraam marked 2 inline comments as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
Files:
clang/docs/UsersManual.rst
clang/include/clang/Basic/FPOptions.def
zahiraam added inline comments.
Comment at: clang/include/clang/Basic/LangOptions.h:301
+ enum Float16ExcessPrecisionKind { FPP_Standard, FPP_Fast, FPP_None };
+
rjmccall wrote:
> rjmccall wrote:
> > You can leave this named `ExcessPrecisionKind` — if we
rjmccall added inline comments.
Comment at: clang/include/clang/Basic/LangOptions.h:301
+ enum Float16ExcessPrecisionKind { FPP_Standard, FPP_Fast, FPP_None };
+
rjmccall wrote:
> You can leave this named `ExcessPrecisionKind` — if we introduce excess
>
zahiraam added a comment.
@rjmccall Would you mind looking at this please? Thanks.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
zahiraam updated this revision to Diff 477179.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
Files:
clang/docs/UsersManual.rst
clang/include/clang/Basic/FPOptions.def
clang/include/clang/Basic/LangOptions.def
zahiraam added inline comments.
Comment at: clang/docs/UsersManual.rst:1732
+.. option:: -fexcess-precision:
+
+ By default, Clang uses excess precision to calculate ``_Float16``
rjmccall wrote:
>
>The C and C++ standards allow floating-point expressions
zahiraam updated this revision to Diff 476913.
zahiraam marked 6 inline comments as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
Files:
clang/docs/UsersManual.rst
clang/include/clang/Basic/FPOptions.def
rjmccall added a comment.
At some point, we're going to have to figure out how this impacts
`FLT_EVAL_METHOD`, but we can do that in a separate patch.
Comment at: clang/docs/UsersManual.rst:1732
+.. option:: -fexcess-precision:
+
+ By default, Clang uses excess precision to
zahiraam updated this revision to Diff 476587.
zahiraam marked 2 inline comments as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
Files:
clang/docs/UsersManual.rst
clang/include/clang/Basic/FPOptions.def
rjmccall added inline comments.
Comment at: clang/include/clang/Basic/LangOptions.def:320
BENIGN_ENUM_LANGOPT(FPEvalMethod, FPEvalMethodKind, 2, FEM_UnsetOnCommandLine,
"FP type used for floating point arithmetic")
+BENIGN_ENUM_LANGOPT(FPPrecisionMode,
zahiraam added inline comments.
Comment at: clang/include/clang/Basic/LangOptions.h:300
+FPP_Fast,
+FPP_None
+ };
zahiraam wrote:
> andrew.w.kaylor wrote:
> > Is FPP_None somehow the same as fexcess-precision=16? If not, what does it
> > mean?
> Yes,
zahiraam updated this revision to Diff 476161.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
Files:
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/Options.td
zahiraam added inline comments.
Comment at: clang/include/clang/Basic/LangOptions.h:300
+FPP_Fast,
+FPP_None
+ };
andrew.w.kaylor wrote:
> Is FPP_None somehow the same as fexcess-precision=16? If not, what does it
> mean?
Yes, maybe this is confusing.
zahiraam updated this revision to Diff 475902.
zahiraam marked 5 inline comments as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
Files:
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
andrew.w.kaylor added a comment.
Can you update the clang user's manual to describe the behavior of this option?
The excess precision issue is also mentioned in the clang language extensions
document
(https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point).
zahiraam added a comment.
@rjmccall Would you mind looking at this when you have a moment? Thanks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136176/new/
https://reviews.llvm.org/D136176
___
32 matches
Mail list logo
