[Flang] Fix build failure on MacOS

[kiranchandramohan]

Summary

std::clock_t can be an unsigned value on some platforms like MacOS and therefore needs a cast when initializing an std::clock_t value with -1.

There is a CI failing in https://github.com/flang-compiler/f18-llvm-project/pull/625/checks?check_run_id=3293634051

Attempted fix in https://github.com/llvm/llvm-project/pull/333 and https://github.com/llvm/llvm-project/pull/334.

Also reported by @AlexisPerry.

[klausler]

flang/runtime/time-intrinsic.cpp

39

You don't need move construction of the result of the cast.

[kiranchandramohan]

Fix comment by @klausler to not use the move constructor.

[erichkeane]

flang/runtime/time-intrinsic.cpp

39

Note this was a function-style cast, which is a no-op here, not a move construction! Still good to omit it however.

[dim]

Thanks. This should be merged to 13.0.0, if possible.

Meanwhile, if I may take a moment of your attention, I ran into this particular error (narrowing {-1} to clock_t) while doing the 13.0.0 rc1 builds for i386-freebsd, i.e. 32-bit x86. Since I encountered a whole lot of other issues where 32 and 64 bit quantities were mixed, does it make sense at all to attempt to build flang on 32-bit systems?

My guess is that this was never a goal, and 64 bit systems (i.e. LP64) are really the only support targets?

[thieta]

In D107972#2941882, @dim wrote:

Thanks. This should be merged to 13.0.0, if possible.

I have asked @tstellar to cherry-pick this over to release/13.x since I ran into this when testing -rc1 on macOS. In the future I recommend you file an issue that is a blocker for the current release so it get's picked up by the release manager.

[kiranchandramohan]

In D107972#2941882, @dim wrote:

Thanks. This should be merged to 13.0.0, if possible.

Meanwhile, if I may take a moment of your attention, I ran into this particular error (narrowing {-1} to clock_t) while doing the 13.0.0 rc1 builds for i386-freebsd, i.e. 32-bit x86. Since I encountered a whole lot of other issues where 32 and 64 bit quantities were mixed, does it make sense at all to attempt to build flang on 32-bit systems?

My guess is that this was never a goal, and 64 bit systems (i.e. LP64) are really the only support targets?

Could @sscalpone or @klausler or @schweitz take this question?

[kiranchandramohan]

In D107972#2943277, @thieta wrote:

In D107972#2941882, @dim wrote:

Thanks. This should be merged to 13.0.0, if possible.

I have asked @tstellar to cherry-pick this over to release/13.x since I ran into this when testing -rc1 on macOS. In the future I recommend you file an issue that is a blocker for the current release so it get's picked up by the release manager.

Sure. Thanks for your help here @thieta.

[kiranchandramohan]

In D107972#2945251, @kiranchandramohan wrote:

In D107972#2941882, @dim wrote:

Thanks. This should be merged to 13.0.0, if possible.

Meanwhile, if I may take a moment of your attention, I ran into this particular error (narrowing {-1} to clock_t) while doing the 13.0.0 rc1 builds for i386-freebsd, i.e. 32-bit x86. Since I encountered a whole lot of other issues where 32 and 64 bit quantities were mixed, does it make sense at all to attempt to build flang on 32-bit systems?

My guess is that this was never a goal, and 64 bit systems (i.e. LP64) are really the only support targets?

Could @sscalpone or @klausler or @schweitz take this question?

@dim I did not get a chance to discuss this with everyone. But my understanding is that at the moment we test only for 64 bit systems. Support for 32 bit systems (I think) is not ruled out but is not of immediate interest.

[dim]

In D107972#2955218, @kiranchandramohan wrote:

In D107972#2945251, @kiranchandramohan wrote:

In D107972#2941882, @dim wrote:

Thanks. This should be merged to 13.0.0, if possible.

Meanwhile, if I may take a moment of your attention, I ran into this particular error (narrowing {-1} to clock_t) while doing the 13.0.0 rc1 builds for i386-freebsd, i.e. 32-bit x86. Since I encountered a whole lot of other issues where 32 and 64 bit quantities were mixed, does it make sense at all to attempt to build flang on 32-bit systems?

My guess is that this was never a goal, and 64 bit systems (i.e. LP64) are really the only support targets?

Could @sscalpone or @klausler or @schweitz take this question?

@dim I did not get a chance to discuss this with everyone. But my understanding is that at the moment we test only for 64 bit systems. Support for 32 bit systems (I think) is not ruled out but is not of immediate interest.

Thank you! This was just out of interest, to see if I could add the flang component to my FreeBSD release builds for 32-bit x86. (For x86_64 I did not encounter build issues in flang itself, except maybe for a few warnings.)

I think it might be quite some effort to make it buildable on 32-bit systems, since I've seen that flang uses many 'index' variables which are declared explicitly as int64_t or uint64_t, and these are freely mixed with size_t's. You'd have to carefully go over all of these to see where you can cleanly convert back and forth...

flang/runtime/time-intrinsic.cpp

 // We will invoke our helper with an integer literal argument, so the overload
 // with the highest priority should have the type int.
 using fallback_implementation = double;
 using preferred_implementation = int;
 
 // This is the fallback implementation, which should work everywhere.
 template <typename Unused = void> double GetCpuTime(fallback_implementation) {
   std::clock_t timestamp{std::clock()};
-  if (timestamp != std::clock_t{-1}) {
+  if (timestamp != static_cast<std::clock_t>(-1)) {
     return static_cast<double>(timestamp) / CLOCKS_PER_SEC;
   }
 
   // Return some negative value to represent failure.
   return -1.0;
 }
 
 // POSIX implementation using clock_gettime. This is only enabled if