Implement reinterpret instructions #459

chfast · 2020-08-04T19:53:19Z

lib/fizzy/execute.cpp

gumb0 · 2020-08-13T12:46:05Z

lib/fizzy/execute.cpp

@@ -357,6 +357,20 @@ inline void convert(OperandStack& stack) noexcept
    stack.top() = static_cast<DstT>(stack.top().as<SrcT>());
 }

+/// Performs a bitcast from SrcT type to DstT type.
+///
+/// This is a formality (except for f32 -> i32 where zero-extension is needed)


What zero-extension?

When we push a i32 value to the stack we actually zero-extend it to i64 in Value.

I improved the explanation.

gumb0 · 2020-08-14T14:53:08Z

lib/fizzy/execute.cpp

+/// Performs a bit_cast from SrcT type to DstT type.
+///
+/// This is a formality and should be optimized to empty function in assembly.
+/// Except for f32 -> i32 where pushing the result i32 value to the stack requires zero-extension.


Suggested change

/// Except for f32 -> i32 where pushing the result i32 value to the stack requires zero-extension.

/// Except for f32 -> i32 where pushing the result i32 value to the stack requires zero-extension to 64-bit.

gumb0 · 2020-08-14T14:56:06Z

lib/fizzy/execute.cpp

@@ -357,6 +357,20 @@ inline void convert(OperandStack& stack) noexcept
    stack.top() = static_cast<DstT>(stack.top().as<SrcT>());
 }

+/// Performs a bit_cast from SrcT type to DstT type.
+///
+/// This is a formality and should be optimized to empty function in assembly.


Nit: I wouldn't call it a "formality". Compiler can optimize it away, but from C++ point of view it's required to avoid UB, right?

That's what I call formality.

codecov · 2020-08-14T15:05:34Z

Codecov Report

Merging #459 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #459   +/-   ##
=======================================
  Coverage   99.55%   99.55%           
=======================================
  Files          54       54           
  Lines       16685    16729   +44     
=======================================
+ Hits        16611    16655   +44     
  Misses         74       74

axic · 2020-08-14T15:45:21Z

lib/fizzy/execute.cpp

+    static_assert(sizeof(SrcT) == sizeof(DstT));
+    const auto src = stack.top().as<SrcT>();
+    DstT dst;
+    __builtin_memcpy(&dst, &src, sizeof(dst));


Why couldn't reinterpret_cast work here instead? Or the memcpy option somehow magically does zero extension?

You cannot do reinterpret_cast<uint32_t>(f32).

Using pointers would work: *reinterpreter_cast<uint32_t*>(&f32), but in theory that is not valid use of reinterpret_cast and undefined behavior. Using memcpy is the safest way of doing type pruning. Although looks ugly.

The C++20 there will be std::bit_cast - exactly what we need here.

chfast force-pushed the fp_reinterpret branch from db4a8e3 to 5d8af8b Compare August 5, 2020 08:20

chfast force-pushed the fp_demote branch 2 times, most recently from f8a6675 to 20cdd1b Compare August 6, 2020 10:34

chfast force-pushed the fp_reinterpret branch from 5d8af8b to 27d62d7 Compare August 6, 2020 10:39

chfast force-pushed the fp_demote branch 4 times, most recently from 0fa4519 to 7d8ad08 Compare August 12, 2020 16:18

chfast force-pushed the fp_reinterpret branch from 27d62d7 to b40bb03 Compare August 12, 2020 16:30

chfast force-pushed the fp_demote branch 2 times, most recently from b5cef53 to 15dba1e Compare August 12, 2020 16:53

chfast force-pushed the fp_reinterpret branch 2 times, most recently from 8582a80 to 4f0e3a4 Compare August 12, 2020 17:43

chfast marked this pull request as ready for review August 12, 2020 17:43

chfast requested review from axic and gumb0 August 12, 2020 17:43

chfast force-pushed the fp_demote branch 2 times, most recently from a94bc35 to 111b100 Compare August 12, 2020 21:02

gumb0 reviewed Aug 13, 2020

View reviewed changes

lib/fizzy/execute.cpp Outdated Show resolved Hide resolved

gumb0 reviewed Aug 13, 2020

View reviewed changes

chfast force-pushed the fp_demote branch 9 times, most recently from 28be58f to d99c375 Compare August 14, 2020 13:32

chfast force-pushed the fp_reinterpret branch 2 times, most recently from 2d3a07a to 0d05d5a Compare August 14, 2020 14:06

chfast force-pushed the fp_demote branch from d99c375 to b0b31f4 Compare August 14, 2020 14:30

Base automatically changed from fp_demote to master August 14, 2020 14:43

chfast force-pushed the fp_reinterpret branch from 0d05d5a to 487a421 Compare August 14, 2020 14:50

gumb0 reviewed Aug 14, 2020

View reviewed changes

gumb0 approved these changes Aug 14, 2020

View reviewed changes

axic reviewed Aug 14, 2020

View reviewed changes

chfast added 2 commits August 14, 2020 18:04

Implement reinterpret instructions

5e0c9d4

ci: Update spectests expectations

ab550e4

chfast force-pushed the fp_reinterpret branch from 487a421 to ab550e4 Compare August 14, 2020 16:04

axic approved these changes Aug 14, 2020

View reviewed changes

chfast merged commit fa0ec8d into master Aug 14, 2020

chfast deleted the fp_reinterpret branch August 14, 2020 16:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement reinterpret instructions #459

Implement reinterpret instructions #459

chfast commented Aug 4, 2020 •

edited

Loading

gumb0 Aug 13, 2020

chfast Aug 14, 2020

chfast Aug 14, 2020

gumb0 Aug 14, 2020

gumb0 Aug 14, 2020

chfast Aug 14, 2020

codecov bot commented Aug 14, 2020

axic Aug 14, 2020 •

edited

Loading

chfast Aug 14, 2020

	/// Except for f32 -> i32 where pushing the result i32 value to the stack requires zero-extension.
	/// Except for f32 -> i32 where pushing the result i32 value to the stack requires zero-extension to 64-bit.

Implement reinterpret instructions #459

Implement reinterpret instructions #459

Conversation

chfast commented Aug 4, 2020 • edited Loading

gumb0 Aug 13, 2020

Choose a reason for hiding this comment

chfast Aug 14, 2020

Choose a reason for hiding this comment

chfast Aug 14, 2020

Choose a reason for hiding this comment

gumb0 Aug 14, 2020

Choose a reason for hiding this comment

gumb0 Aug 14, 2020

Choose a reason for hiding this comment

chfast Aug 14, 2020

Choose a reason for hiding this comment

codecov bot commented Aug 14, 2020

Codecov Report

axic Aug 14, 2020 • edited Loading

Choose a reason for hiding this comment

chfast Aug 14, 2020

Choose a reason for hiding this comment

chfast commented Aug 4, 2020 •

edited

Loading

axic Aug 14, 2020 •

edited

Loading