[ppc64] Use signed integers for machine ops tests [v8/v8 : main]
Clemens Backes (Gerrit)
Attention is currently required from: Vasili Skurydzin, Yang Guo, Milad Fa.
Patch set 2:Code-Review +1
To view, visit change 2968449. To unsubscribe, or for help writing mail filters, visit settings.
Vasili Skurydzin (Gerrit)
Attention is currently required from: Clemens Backes, Milad Fa.
1 comment:
Patchset:
Hello! Just wondering: Any plans to add Uint32Constant data type as an alternative to Int32Constant?
To view, visit change 2968449. To unsubscribe, or for help writing mail filters, visit settings.
Clemens Backes (Gerrit)
Attention is currently required from: Vasili Skurydzin, Milad Fa.
1 comment:
Patchset:
Hello! Just wondering: Any plans to add Uint32Constant data type as an alternative to Int32Constant?
Georg, can you answer this, or provide background why we only have Int32Constant?
To view, visit change 2968449. To unsubscribe, or for help writing mail filters, visit settings.
Milad Fa (Gerrit)
Attention is currently required from: Vasili Skurydzin.
To view, visit change 2968449. To unsubscribe, or for help writing mail filters, visit settings.
Vasili Skurydzin (Gerrit)
Attention is currently required from: Vasili Skurydzin.
Patch set 2:Commit-Queue +2
V8 LUCI CQ (Gerrit)
V8 LUCI CQ submitted this change.
[ppc64] Use signed integers for machine ops tests
When result is returned in a register to the calling code, some GCC
versions use 32 bit compare, and some use 64 bit compare. In the case
comparison is 64 bit, GCC on PPC64 arch is expecting the return value to
be sign-extended, leading to an error in comparison.
Change-Id: I05b7e1566bc9bb931ce9998bb310eb29c50e90e4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2968449
Reviewed-by: Clemens Backes <clem...@chromium.org>
Reviewed-by: Milad Fa <mfar...@redhat.com>
Commit-Queue: Vasili Skurydzin <vasili.s...@ibm.com>
Cr-Commit-Position: refs/heads/master@{#75245}
---
M test/cctest/compiler/test-run-machops.cc
1 file changed, 14 insertions(+), 14 deletions(-)
Georg Neis (Gerrit)
1 comment:
Patchset:
Georg, can you answer this, or provide background why we only have Int32Constant?
I'm not aware of such plans, and I don't have any historical background on this. I guess it just wasn't needed.
I'd like to understand the problem behind this CL. Is this a bug in some GCC versions? Or a bug in our code generator? Or something else? The code that we generate on x64 sign-extends the 32-bit integer. I just had a look at recent clang and gcc on x64, they zero-extend instead.
To view, visit change 2968449. To unsubscribe, or for help writing mail filters, visit settings.
Vasili Skurydzin (Gerrit)
1 comment:
Patchset:
I'm not aware of such plans, and I don't have any historical background on this. […]
Hello! This is not a bug in either - only a specific way GCC does things (according to GCC maintainers).
On ppc64, gcc sometimes (with optimization enabled) expects the register that carries 32 bit return value to be sign extended to 64 bit because it is easier to optimize (even if the value is unsigned).
The usage of uint32_t (instead of int32_t) in this case have caused such optimization to happen - despite carrying 32bit unsigned value, return register gets sign extended on ppc64.
Using int32_t return value removes this issue.
To view, visit change 2968449. To unsubscribe, or for help writing mail filters, visit settings.