Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

abseil-duration-factory-scale test fails on 32-bit IA32 #40805

Open
llvmbot opened this issue Apr 11, 2019 · 1 comment
Open

abseil-duration-factory-scale test fails on 32-bit IA32 #40805

llvmbot opened this issue Apr 11, 2019 · 1 comment
Labels
bugzilla Issues migrated from bugzilla clang-tidy regression

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 11, 2019

Bugzilla Link 41460
Version unspecified
OS Linux
Attachments clang build log on Archlinux32
Reporter LLVM Bugzilla Contributor
CC @EugeneZelenko

Extended Description

The clang-tidy testsuite fails on 32-bit IA32 for abseil-duration-factory-scale:

Failing Tests (1):
Clang Tools :: clang-tidy/abseil-duration-factory-scale.cpp

Expected Passes : 1126
Expected Failures : 1
Unsupported Tests : 3
Unexpected Failures: 1

FAILED: tools/extra/test/CMakeFiles/check-clang-tools
cd /build/clang/src/cfe-8.0.0.src/build/tools/extra/test && /usr/bin/python /usr/bin/lit -sv /build/clang/src/cfe-8.0.0.src/build/tools/extra/test
ninja: build stopped: subcommand failed.

The full build log is attached.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@kabe-gh
Copy link

kabe-gh commented Oct 11, 2022

I am also having trouble getting abseil-duration-factory-scale.cpp check on 32bit fail.

I tracked this down to clang-tidy/abseil/DurationFactoryScaleCheck.cpp:getNewScale()
comparing double float value to 1.0,
but 0.001 * 1000 doesn't equal 1.0 due to precision.

Following patch seems to fix this.
I don't know why x86_64 is working without this kind of patch.

Subject: fuzzy match double Multiplier == 1.0 in DurationFactoryScaleCheck.cpp
From: <kabe@>

Replace double comparison d == 1.0 with fuzzy match std::abs(d - 1.0) < 1e-14.
1e-14 is an arbitary epsilon.

Without this patch, clang-tidy did not downshift
  d = absl::Seconds(1e-3 * 30);
to
  d = absl::Milliseconds(30);


diff -up ./clang-tools-extra-13.0.1.src/clang-tidy/abseil/DurationFactoryScaleCheck.cpp.t64 ./clang-tools-extra-13.0.1.src/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
--- ./clang-tools-extra-13.0.1.src/clang-tidy/abseil/DurationFactoryScaleCheck.cpp.t64  2022-01-21 06:31:59.000000000 +0900
+++ ./clang-tools-extra-13.0.1.src/clang-tidy/abseil/DurationFactoryScaleCheck.cpp      2022-10-11 22:27:36.694627456 +0900
@@ -96,12 +96,15 @@ getNewScaleSingleStep(DurationScale OldS
 // would produce a new scale.  If so, return it, otherwise `None`.
 static llvm::Optional<DurationScale> getNewScale(DurationScale OldScale,
                                                  double Multiplier) {
-  while (Multiplier != 1.0) {
+  //while (Multiplier != 1.0)
+  while (std::abs(Multiplier - 1.0) > 1e-14) {
     llvm::Optional<std::tuple<DurationScale, double>> Result =
         getNewScaleSingleStep(OldScale, Multiplier);
     if (!Result)
       break;
-    if (std::get<1>(*Result) == 1.0)
+    //if (std::get<1>(*Result) == 1.0)
+    //Above does not work properly when shifting up 0.001 to 1.0
+    if (std::abs(std::get<1>(*Result) - 1.0) < 1e-14)
       return std::get<0>(*Result);
     Multiplier = std::get<1>(*Result);
     OldScale = std::get<0>(*Result);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang-tidy regression
Projects
None yet
Development

No branches or pull requests

3 participants