Fix build on FreeBSD/powerpc #616

kgotlinux · 2020-02-10T13:04:38Z

Only glibc has __ppc_get_timebase(), __ppc_get_timebase_freq() and sys/platform/ppc.h. For TBR and TBR frequency, FreeBSD doesn't have equivalent functions.

Only glibc has __ppc_get_timebase(), __ppc_get_timebase_freq() and sys/platform/ppc.h.

absl/base/internal/unscaledcycleclock.cc

derekmauro · 2020-02-10T21:27:54Z

absl/base/internal/unscaledcycleclock.cc

  return __ppc_get_timebase();
+#elif defined(__FreeBSD__)
+  union { long long complete; unsigned int part[2]; } ticks;


Though you sometimes see it, type-punning through a union is undefined behavior in C++. See https://adriann.github.io/undefined_behavior.html for example.

derekmauro · 2020-02-10T21:32:35Z

absl/base/internal/unscaledcycleclock.cc

+ #elif defined(__FreeBSD__)
+  double timebaseFrequency = 0;
+  size_t length = sizeof(timebaseFrequency);
+  sysctlbyname("kern.timecounter.tc.timebase.frequency", &timebaseFrequency, &length, NULL, 0);


Can you cache this value using some sort of "once" initialization? I don't think Abseil calls the code at all, but other (internal) code calls it very often.

For various reasons you'll need to use absl::base_internal::LowLevelCallOnce in this context.

abseil-cpp/absl/base/call_once.h

Lines 199 to 208 in bf78e97

template <typename Callable, typename... Args>

void LowLevelCallOnce(absl::once_flag* flag, Callable&& fn, Args&&... args) {

std::atomic<uint32_t>* once = base_internal::ControlWord(flag);

uint32_t s = once->load(std::memory_order_acquire);

if (ABSL_PREDICT_FALSE(s != base_internal::kOnceDone)) {

base_internal::CallOnceImpl(once, base_internal::SCHEDULE_KERNEL_ONLY,

std::forward<Callable>(fn),

std::forward<Args>(args)...);

}

}

derekmauro · 2020-02-10T21:33:32Z

absl/base/internal/unscaledcycleclock.cc

  return __ppc_get_timebase_freq();
+ #elif defined(__FreeBSD__)
+  double timebaseFrequency = 0;


https://google.github.io/styleguide/cppguide.html#Variable_Names

Variable names are lower-case with underscore in Google C++ style.

derekmauro · 2020-02-10T21:36:41Z

absl/base/internal/unscaledcycleclock.cc

  return __ppc_get_timebase();
+#elif defined(__FreeBSD__)
+  union { long long complete; unsigned int part[2]; } ticks;


https://google.github.io/styleguide/cppguide.html#Integer_Types

This is a case where you should use fixed-width integer types like int64_t.

1. Add optimized version for powerpc64. 2. Cache timebase_frequency.

kgotlinux · 2020-02-12T18:42:32Z

Please check if it's ok now.

absl/base/internal/unscaledcycleclock.cc

derekmauro · 2020-02-25T21:27:00Z

I pushed changes to your branch to resolve the comments.

- 0033c9e Fix build on FreeBSD/powerpc (#616) by kgotlinux <[email protected]> - 0d5ce27 Export of internal Abseil changes by Abseil Team <[email protected]> - b69c7d8 Export of internal Abseil changes by Abseil Team <[email protected]> - 2a5633f Merge "Export of internal Abseil changes" by Xiaoyi Zhang <[email protected]> - f9b3d6e Add RISCV support to GetProgramCounter() (#621) by Khem Raj <[email protected]> - 0232c87 Add missing ABSL_HAVE_VDSO_SUPPORT conditional (#622) by Sinan Kaya <[email protected]> - 3c81410 Export of internal Abseil changes by Abseil Team <[email protected]> - c44657f Export of internal Abseil changes by Abseil Team <[email protected]> - 98eb410 Export of internal Abseil changes by Abseil Team <[email protected]> - bf78e97 Export of internal Abseil changes by Abseil Team <[email protected]> - d95d156 Export of internal Abseil changes by Abseil Team <[email protected]> - 24713a7 Export of internal Abseil changes by Abseil Team <[email protected]> - 72382c2 Export of internal Abseil changes by Abseil Team <[email protected]> - 08a7e7b Export of internal Abseil changes by Abseil Team <[email protected]> - 36bcd95 Fix pointer format specifier in documentation (#614) by Andre Nguyen <[email protected]> - 0f86336 Export of internal Abseil changes by Abseil Team <[email protected]> - c512f11 Export of internal Abseil changes by Abseil Team <[email protected]> - 37dd256 Export of internal Abseil changes by Abseil Team <[email protected]> - 4442770 fix: Add support for more ARM processors detection (#608) by Andre Nguyen <[email protected]> - 159bf2b Export of internal Abseil changes by Abseil Team <[email protected]> - a2e6ade Use https links. (#586) by nlewycky <[email protected]> - 564001a Export of internal Abseil changes by Abseil Team <[email protected]> - b3aaac8 Export of internal Abseil changes by Abseil Team <[email protected]> - 63ee2f8 Export of internal Abseil changes by Abseil Team <[email protected]> - a048203 Export of internal Abseil changes by Abseil Team <[email protected]> - 1de0166 Export of internal Abseil changes by Abseil Team <[email protected]> - ad904b6 Export of internal Abseil changes by Abseil Team <[email protected]> - 2923513 Export of internal Abseil changes by Abseil Team <[email protected]> - bf86cfe Export of internal Abseil changes by Abseil Team <[email protected]> - 12bc53e Export of internal Abseil changes by Abseil Team <[email protected]> - 1e39f86 Export of internal Abseil changes by Abseil Team <[email protected]> - 77f8700 Export of internal Abseil changes by Abseil Team <[email protected]> - d659fe5 Export of internal Abseil changes by Abseil Team <[email protected]> - a4b757b Export of internal Abseil changes by Abseil Team <[email protected]> - 0514227 Export of internal Abseil changes by Abseil Team <[email protected]> - 7f4fe64 Export of internal Abseil changes by Abseil Team <[email protected]> - 16d9fd5 Export of internal Abseil changes by Abseil Team <[email protected]> - bcaae60 Export of internal Abseil changes by Abseil Team <[email protected]> - 8ba96a8 Export of internal Abseil changes by Abseil Team <[email protected]> - 2103fd9 Export of internal Abseil changes by Abseil Team <[email protected]> - 3df7b52 Export of internal Abseil changes by Abseil Team <[email protected]> - fa8c751 Export of internal Abseil changes by Abseil Team <[email protected]> - 85092b4 Fix Conan builds (#400) by Adrian Ostrowski <[email protected]> - e96ae22 Export of internal Abseil changes by Abseil Team <[email protected]> - 20de2db Export of internal Abseil changes by Abseil Team <[email protected]> - 846e5db Export of internal Abseil changes by Abseil Team <[email protected]> - 8207907 Export of internal Abseil changes by Abseil Team <[email protected]> - 078b89b Export of internal Abseil changes by Abseil Team <[email protected]> - 19b021c Export of internal Abseil changes by Abseil Team <[email protected]> - ecc0033 Always enable proper symbolize implementation on Windows ... by Loo Rong Jie <[email protected]> - 2796d50 Export of internal Abseil changes by Abseil Team <[email protected]> - e4c8d0e Export of internal Abseil changes by Abseil Team <[email protected]> - a15364c Export of internal Abseil changes by Abseil Team <[email protected]> - ab3552a Export of internal Abseil changes by Abseil Team <[email protected]> - e9f9000 Fix ABSL_WAITER_MODE detection for mingw (#342) by Joe Sylve <[email protected]> - abea769 Fix ABSL_HAVE_ALARM check on mingw (#341) by Joe Sylve <[email protected]> - 25597bd Export of internal Abseil changes by Abseil Team <[email protected]> - aad33fe Export of internal Abseil changes by Abseil Team <[email protected]> - 8fe7214 Export of internal Abseil changes by Abseil Team <[email protected]> - debac94 Export of internal Abseil changes by Abseil Team <[email protected]> - 882b350 Fix spelling errors (#384) by Sungmann Cho <[email protected]> - 502efe6 Export of internal Abseil changes by Abseil Team <[email protected]> - ccdd1d5 Export of internal Abseil changes by Abseil Team <[email protected]> - ddf8e52 Export of internal Abseil changes by Abseil Team <[email protected]> - 6ec1362 Export of internal Abseil changes by Abseil Team <[email protected]> - ac78ffc Export of internal Abseil changes by Abseil Team <[email protected]> - 5374c56 Export of internal Abseil changes by Abseil Team <[email protected]> - 97c1664 Export of internal Abseil changes by Abseil Team <[email protected]> - 325fd7b Export of internal Abseil changes by Abseil Team <[email protected]> - 83c1d65 Export of internal Abseil changes by Abseil Team <[email protected]> - eb6b7bd Export of internal Abseil changes by Abseil Team <[email protected]> - 9ddac55 Export of internal Abseil changes by Abseil Team <[email protected]> - 1948f6f Export of internal Abseil changes by Abseil Team <[email protected]> - a0d1e09 Export of internal Abseil changes by Abseil Team <[email protected]> - 2d2d7fb Export of internal Abseil changes by Abseil Team <[email protected]> - 0302d1e supppress unused variable warning for gcc (#372) by Martin <[email protected]> - 262d74b Export of internal Abseil changes by Abseil Team <[email protected]> - f0afae0 Export of internal Abseil changes by Abseil Team <[email protected]> - 0e7afdc Export of internal Abseil changes by Abseil Team <[email protected]> - 9a41ffd Export of internal Abseil changes by Abseil Team <[email protected]> - 36910d3 [bazel] Add fixes for --incompatible_load_cc_rules_from_b... by Yannic <[email protected]> - aae8143 Export of internal Abseil changes by Abseil Team <[email protected]> - d9aa92d Export of internal Abseil changes by Abseil Team <[email protected]> - 321ab53 Export of internal Abseil changes by Abseil Team <[email protected]> - 4ef5740 Export of internal Abseil changes by Abseil Team <[email protected]> GitOrigin-RevId: 0033c9e Change-Id: I8a2b70063cb3ab40c6943a6db0fe40cae71ed8d7

-- 09c1e7877210fe85c43631538303af801c233e89 by Gennadiy Rozental <[email protected]>: Change CordRep::length to size_t to be compatible with ChunkIterator. PiperOrigin-RevId: 297901255 -- 226d5e79fb4e20fb09d75f034624d5be770f4ece by Derek Mauro <[email protected]>: Makes absl::string_view::substr constexpr for std::string_view compatibility Fixes #627 PiperOrigin-RevId: 297872778 -- 851aa24a22d0ba5552098bf7e5735c95e4a8d4f7 by Abseil Team <[email protected]>: Reformat one line. PiperOrigin-RevId: 297839574 -- 4f449c462583797455375fa6f1365a6b2cfa7e0a by Benjamin Barenblat <[email protected]>: Internal change PiperOrigin-RevId: 297677173 -- 2d41a250e9a8f272946bc3262463e4025d88fba3 by Abseil Team <[email protected]>: Internal change PiperOrigin-RevId: 297537076 -- 6e7fbe1e90999a58556d41955bef44033c61876c by Gennadiy Rozental <[email protected]>: Internal change PiperOrigin-RevId: 297376994 -- 5acf79338191b31cec158b06cd96ca8dfa3e81fe by Derek Mauro <[email protected]>: Add a debug-mode bounds-check on absl::Span::operator[]. PiperOrigin-RevId: 297355826 -- 1c540d06a56c7e92bb07b90f16b4e00b014ef18f by CJ Johnson <[email protected]>: Adding new LTS to the list PiperOrigin-RevId: 297235265 -- 696ce48bea6927436ff89f59b887e5869b1b0f38 by Derek Mauro <[email protected]>: Fix build on FreeBSD/powerpc (implement UnscaledCycleClock) Merges/Fixes GitHub #616 PiperOrigin-RevId: 297188640 GitOrigin-RevId: 09c1e7877210fe85c43631538303af801c233e89 Change-Id: I5d97b16bb6378792d2fcf7d29080cca18aa7729a

* Fix build on FreeBSD/powerpc Only glibc has __ppc_get_timebase(), __ppc_get_timebase_freq() and sys/platform/ppc.h.

-- 09c1e7877210fe85c43631538303af801c233e89 by Gennadiy Rozental <[email protected]>: Change CordRep::length to size_t to be compatible with ChunkIterator. PiperOrigin-RevId: 297901255 -- 226d5e79fb4e20fb09d75f034624d5be770f4ece by Derek Mauro <[email protected]>: Makes absl::string_view::substr constexpr for std::string_view compatibility Fixes abseil#627 PiperOrigin-RevId: 297872778 -- 851aa24a22d0ba5552098bf7e5735c95e4a8d4f7 by Abseil Team <[email protected]>: Reformat one line. PiperOrigin-RevId: 297839574 -- 4f449c462583797455375fa6f1365a6b2cfa7e0a by Benjamin Barenblat <[email protected]>: Internal change PiperOrigin-RevId: 297677173 -- 2d41a250e9a8f272946bc3262463e4025d88fba3 by Abseil Team <[email protected]>: Internal change PiperOrigin-RevId: 297537076 -- 6e7fbe1e90999a58556d41955bef44033c61876c by Gennadiy Rozental <[email protected]>: Internal change PiperOrigin-RevId: 297376994 -- 5acf79338191b31cec158b06cd96ca8dfa3e81fe by Derek Mauro <[email protected]>: Add a debug-mode bounds-check on absl::Span::operator[]. PiperOrigin-RevId: 297355826 -- 1c540d06a56c7e92bb07b90f16b4e00b014ef18f by CJ Johnson <[email protected]>: Adding new LTS to the list PiperOrigin-RevId: 297235265 -- 696ce48bea6927436ff89f59b887e5869b1b0f38 by Derek Mauro <[email protected]>: Fix build on FreeBSD/powerpc (implement UnscaledCycleClock) Merges/Fixes GitHub abseil#616 PiperOrigin-RevId: 297188640 GitOrigin-RevId: 09c1e7877210fe85c43631538303af801c233e89 Change-Id: I5d97b16bb6378792d2fcf7d29080cca18aa7729a

kgotlinux added 2 commits February 10, 2020 13:40

Fix build on FreeBSD/powerpc

d9d1e00

Only glibc has __ppc_get_timebase(), __ppc_get_timebase_freq() and sys/platform/ppc.h.

Update unscaledcycleclock.cc

8e3a121

googlebot added the cla: yes label Feb 10, 2020

mbxx assigned derekmauro Feb 10, 2020

derekmauro reviewed Feb 10, 2020

View reviewed changes

absl/base/internal/unscaledcycleclock.cc Show resolved Hide resolved

derekmauro requested changes Feb 10, 2020

View reviewed changes

Fix problems pointed out in PR

3e13ca0

1. Add optimized version for powerpc64. 2. Cache timebase_frequency.

derekmauro requested changes Feb 20, 2020

View reviewed changes

absl/base/internal/unscaledcycleclock.cc Outdated Show resolved Hide resolved

absl/base/internal/unscaledcycleclock.cc Outdated Show resolved Hide resolved

absl/base/internal/unscaledcycleclock.cc Show resolved Hide resolved

Tweaks from code review

857764b

derekmauro merged commit 0033c9e into abseil:master Feb 25, 2020

rongjiecomputer pushed a commit to rongjiecomputer/abseil-cpp that referenced this pull request Oct 8, 2020

Fix build on FreeBSD/powerpc (abseil#616)

c6bdcf7

* Fix build on FreeBSD/powerpc Only glibc has __ppc_get_timebase(), __ppc_get_timebase_freq() and sys/platform/ppc.h.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix build on FreeBSD/powerpc #616

Fix build on FreeBSD/powerpc #616

kgotlinux commented Feb 10, 2020

derekmauro Feb 10, 2020

derekmauro Feb 10, 2020

derekmauro Feb 10, 2020

derekmauro Feb 10, 2020

kgotlinux commented Feb 12, 2020

derekmauro commented Feb 25, 2020

	template <typename Callable, typename... Args>
	void LowLevelCallOnce(absl::once_flag* flag, Callable&& fn, Args&&... args) {
	std::atomic<uint32_t>* once = base_internal::ControlWord(flag);
	uint32_t s = once->load(std::memory_order_acquire);
	if (ABSL_PREDICT_FALSE(s != base_internal::kOnceDone)) {
	base_internal::CallOnceImpl(once, base_internal::SCHEDULE_KERNEL_ONLY,
	std::forward<Callable>(fn),
	std::forward<Args>(args)...);
	}
	}

Fix build on FreeBSD/powerpc #616

Fix build on FreeBSD/powerpc #616

Conversation

kgotlinux commented Feb 10, 2020

derekmauro Feb 10, 2020

Choose a reason for hiding this comment

derekmauro Feb 10, 2020

Choose a reason for hiding this comment

derekmauro Feb 10, 2020

Choose a reason for hiding this comment

derekmauro Feb 10, 2020

Choose a reason for hiding this comment

kgotlinux commented Feb 12, 2020

derekmauro commented Feb 25, 2020