Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/42504 )

Change subject: arch-arm,base,dev: Eliminate the power() function from intmath.hh.
......................................................................

arch-arm,base,dev: Eliminate the power() function from intmath.hh.

This function causes problems with gcc 5 which incorrectly complains
about the call to warn_if inside a constexpr function. That should only
be an error if a call to a non-constexpr is unavoidable, and even then
the compiler isn't required to emit a diagnostic.

Rather than drop the warning, or add ifdefs to deal with these defective
versions of gcc, this change eliminates the power() function entirely.
Most inputs to this function would overflow anyway, which is reportedly
why no integer version of an exponentiation function is defined in the
standard library, and all uses of this function can easily and more
efficiently be replaced by simple left and right shifts.

Finally, by eliminating the power() function, we also remove the
dependence on base/logging.hh.

Change-Id: I4d014163883d12db46da4ee752696c8225534ee8
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42504
Reviewed-by: Gabe Black <[email protected]>
Maintainer: Gabe Black <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/arch/arm/isa/insts/data64.isa
M src/base/intmath.hh
M src/base/intmath.test.cc
M src/dev/arm/timer_cpulocal.cc
M src/dev/arm/timer_sp804.cc
5 files changed, 8 insertions(+), 36 deletions(-)

Approvals:
  Gabe Black: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/isa/insts/data64.isa b/src/arch/arm/isa/insts/data64.isa
index 1cd17b7..46a046b 100644
--- a/src/arch/arm/isa/insts/data64.isa
+++ b/src/arch/arm/isa/insts/data64.isa
@@ -382,7 +382,7 @@
            Request::Flags memAccessFlags = Request::CACHE_BLOCK_ZERO;
            EA = XBase;
            assert(!(Dczid & 0x10));
-           uint64_t op_size = power(2, Dczid + 2);
+           uint64_t op_size = 1ULL << (Dczid + 2);
            EA &= ~(op_size - 1);

    '''
diff --git a/src/base/intmath.hh b/src/base/intmath.hh
index ba20177..85ee525 100644
--- a/src/base/intmath.hh
+++ b/src/base/intmath.hh
@@ -46,31 +46,11 @@
 #include <type_traits>

 #include "base/bitfield.hh"
-#include "base/logging.hh"
 #include "base/types.hh"

 /**
  * @ingroup api_base_utils
  */
-static constexpr uint64_t
-power(uint32_t n, uint32_t e)
-{
-    uint64_t result = 1;
-    uint64_t component = n;
-    while (e) {
-        uint64_t last = result;
-        if (e & 0x1)
-            result *= component;
-        warn_if(result < last, "power() overflowed!");
-        e >>= 1;
-        component *= component;
-    }
-    return result;
-}
-
-/**
- * @ingroup api_base_utils
- */
 template <class T>
 static constexpr std::enable_if_t<std::is_integral<T>::value, int>
 floorLog2(T x)
diff --git a/src/base/intmath.test.cc b/src/base/intmath.test.cc
index e953a7e..b4bd2a5 100644
--- a/src/base/intmath.test.cc
+++ b/src/base/intmath.test.cc
@@ -54,13 +54,6 @@
     EXPECT_FALSE(isPowerOf2(1679616));
 }

-TEST(IntmathTest, power)
-{
-    EXPECT_EQ(65536, power(2, 16));
-    EXPECT_EQ(9765625, power(5, 10));
-    EXPECT_EQ(43046721, power(power(3, 4), 4));
-}
-
 TEST(IntmathTest, floorLog2)
 {
     EXPECT_EQ(0, floorLog2(1));
diff --git a/src/dev/arm/timer_cpulocal.cc b/src/dev/arm/timer_cpulocal.cc
index 760adf5..3b120f6 100644
--- a/src/dev/arm/timer_cpulocal.cc
+++ b/src/dev/arm/timer_cpulocal.cc
@@ -123,8 +123,7 @@
                 timerZeroEvent.when(), parent->clockPeriod(),
                 timerControl.prescalar);
         time = timerZeroEvent.when() - curTick();
-        time = time / parent->clockPeriod() /
-            power(16, timerControl.prescalar);
+ time = (time / parent->clockPeriod()) >> (4 * timerControl.prescalar);
         DPRINTF(Timer, "-- returning counter at %d\n", time);
         pkt->setLE<uint32_t>(time);
         break;
@@ -143,8 +142,8 @@
                 watchdogZeroEvent.when(), parent->clockPeriod(),
                 watchdogControl.prescalar);
         time = watchdogZeroEvent.when() - curTick();
-        time = time / parent->clockPeriod() /
-            power(16, watchdogControl.prescalar);
+        time = (time / parent->clockPeriod()) >>
+            (4 * watchdogControl.prescalar);
         DPRINTF(Timer, "-- returning counter at %d\n", time);
         pkt->setLE<uint32_t>(time);
         break;
@@ -269,7 +268,7 @@
     if (!timerControl.enable)
         return;

-    Tick time = parent->clockPeriod() * power(16, timerControl.prescalar);
+    Tick time = parent->clockPeriod() << (4 * timerControl.prescalar);
     time *= val;

     if (timerZeroEvent.scheduled()) {
@@ -287,7 +286,7 @@
     if (!watchdogControl.enable)
         return;

- Tick time = parent->clockPeriod() * power(16, watchdogControl.prescalar);
+    Tick time = parent->clockPeriod() << (4 * watchdogControl.prescalar);
     time *= val;

     if (watchdogZeroEvent.scheduled()) {
diff --git a/src/dev/arm/timer_sp804.cc b/src/dev/arm/timer_sp804.cc
index 6403a77..f69da13 100644
--- a/src/dev/arm/timer_sp804.cc
+++ b/src/dev/arm/timer_sp804.cc
@@ -96,7 +96,7 @@
                 zeroEvent.when(), clock, control.timerPrescale);
         Tick time;
         time = zeroEvent.when() - curTick();
-        time = time / clock / power(16, control.timerPrescale);
+        time = (time / clock) >> (4 * control.timerPrescale);
         DPRINTF(Timer, "-- returning counter at %d\n", time);
         pkt->setLE<uint32_t>(time);
         break;
@@ -182,7 +182,7 @@
     if (!control.timerEnable)
         return;

-    Tick time = clock * power(16, control.timerPrescale);
+    Tick time = clock << (4 * control.timerPrescale);
     if (control.timerSize)
         time *= val;
     else

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42504
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I4d014163883d12db46da4ee752696c8225534ee8
Gerrit-Change-Number: 42504
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Giacomo Travaglini <[email protected]>
Gerrit-Reviewer: Kyle Roarty <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to