Skip to content

Conversation

tomeksowi
Copy link
Member

@tomeksowi tomeksowi commented Feb 15, 2024

Remove unnecessary assertion introduced in #98226. It fired a false positive when W^X was enabled.

Part of #84834
cc @wscho77 @HJLeee @clamp03 @JongHeonChoi @t-mustafin @gbalykov @viewizard @ashaurtaev @brucehoult @sirntar @yurai007 @Bajtazar @bartlomiejko

It fired a false positive when W^X was enabled
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 15, 2024
@ghost
Copy link

ghost commented Feb 15, 2024

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

Remove unnecessary assertion. It fired a false positive when W^X was enabled.

Also, reuse emitOutputLong instead of creating our own for RISC-V.

Part of #84834
cc @wscho77 @HJLeee @clamp03 @JongHeonChoi @t-mustafin @gbalykov @viewizard @ashaurtaev @brucehoult @sirntar @yurai007 @Bajtazar

Author: tomeksowi
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@clamp03 clamp03 added the arch-riscv Related to the RISC-V architecture label Feb 15, 2024
@ryujit-bot
Copy link

Diff results for #98484

Throughput diffs

Throughput diffs for linux/arm64 ran on windows/x64

MinOpts (-0.01% to +0.00%)
Collection PDIFF
libraries.pmi.linux.arm64.checked.mch -0.01%

Throughput diffs for osx/arm64 ran on windows/x64

MinOpts (-0.00% to +0.01%)
Collection PDIFF
realworld.run.osx.arm64.checked.mch +0.01%

Throughput diffs for windows/arm64 ran on windows/x64

MinOpts (-0.01% to +0.00%)
Collection PDIFF
realworld.run.windows.arm64.checked.mch -0.01%

Details here


There shouldn't be a problem when we introduce compressed instructions, VF2 supports unaligned stores (sw)
Also, replace compile-time conditions with static_asserts.
@ryujit-bot
Copy link

Diff results for #98484

Throughput diffs

Throughput diffs for osx/arm64 ran on windows/x64

MinOpts (-0.00% to +0.01%)
Collection PDIFF
realworld.run.osx.arm64.checked.mch +0.01%

Throughput diffs for windows/arm64 ran on windows/x64

MinOpts (-0.01% to +0.00%)
Collection PDIFF
realworld.run.windows.arm64.checked.mch -0.01%

Details here


@ryujit-bot
Copy link

Diff results for #98484

Throughput diffs

Throughput diffs for osx/arm64 ran on windows/x64

MinOpts (-0.00% to +0.01%)
Collection PDIFF
realworld.run.osx.arm64.checked.mch +0.01%

Throughput diffs for windows/arm64 ran on windows/x64

MinOpts (-0.00% to +0.01%)
Collection PDIFF
libraries.pmi.windows.arm64.checked.mch +0.01%

Details here


@tomeksowi tomeksowi marked this pull request as ready for review February 15, 2024 21:36
@clamp03 clamp03 requested a review from jakobbotsch February 16, 2024 01:36
Copy link

@bartlomiejko bartlomiejko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ryujit-bot
Copy link

Diff results for #98484

Throughput diffs

Throughput diffs for linux/arm64 ran on windows/x64

MinOpts (-0.01% to +0.00%)
Collection PDIFF
libraries.pmi.linux.arm64.checked.mch -0.01%
realworld.run.linux.arm64.checked.mch -0.01%

Details here


@brucehoult
Copy link

LGTM

@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-riscv Related to the RISC-V architecture area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants