Page MenuHomePhabricator

[XRay] Add generic patchable-function-entry NOP sled implementation
Needs ReviewPublic

Authored by duck-37 on Feb 10 2023, 7:33 PM.

Details

Reviewers
MaskRay
Summary

This patch generalizes the logic for patchable-function-entry=n NOP sleds, meaning that this attribute now has consistent behavior across all architectures that support XRay. The differences in codegen was noticed while porting a separate test-case over to ARM32.

Closes https://github.com/llvm/llvm-project/issues/60672

Diff Detail

Unit TestsFailed

TimeTest
60,170 msx64 debian > Clang.Driver::arm-cortex-cpus-1.c
Script: -- : 'RUN: at line 7'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -target arm -mcpu=generic -### -c /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/arm-cortex-cpus-1.c 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck -check-prefix=CHECK-GENERIC /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/arm-cortex-cpus-1.c
60,370 msx64 debian > Clang.Driver::arm-cortex-cpus-2.c
Script: -- : 'RUN: at line 8'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -target armv8a-linux-eabi -mcpu=cortex-a53+fp16 -### -c /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/arm-cortex-cpus-2.c 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix CHECK-CORTEX-A53-FP16 /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/arm-cortex-cpus-2.c
60,130 msx64 debian > Clang.Driver::crash-report.cpp
Script: -- : 'RUN: at line 1'; rm -rf /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/test/Driver/Output/crash-report.cpp.tmp
60,250 msx64 debian > Clang.Driver::emit-reproducer.c
Script: -- : 'RUN: at line 1'; rm -rf /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/test/Driver/Output/emit-reproducer.c.tmp && mkdir /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/test/Driver/Output/emit-reproducer.c.tmp
60,430 msx64 debian > Clang.Driver::fsanitize.c
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang --target=x86_64-linux-gnu -fsanitize=undefined -fsanitize-trap=undefined /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/fsanitize.c -### 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/fsanitize.c --check-prefix=CHECK-UNDEFINED-TRAP
View Full Test Results (7 Failed)

Event Timeline

duck-37 created this revision.Feb 10 2023, 7:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2023, 7:33 PM
duck-37 requested review of this revision.Feb 10 2023, 7:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2023, 7:33 PM
duck-37 retitled this revision from [XRay] Make patchable-function-entry NOP sled implementation target-independent This patch generalizes the logic for patchable-function-entry NOP sleds, which extends support for them to all architectures that support XRay. A consequence of this... to [XRay] Make patchable-function-entry NOP sled implementation target-independent.Feb 10 2023, 7:34 PM
duck-37 edited the summary of this revision. (Show Details)
duck-37 edited the summary of this revision. (Show Details)

The increased instruction count for x86 is a slight performance regression. Can it be fixed?

Fixes https://github.com/llvm/llvm-project/issues/60672

It's not a bug so I'd use Close. Note: without runtime support, the compiler codegen change isn't really useful

Fixes https://github.com/llvm/llvm-project/issues/60672

It's not a bug so I'd use Close. Note: without runtime support, the compiler codegen change isn't really useful

Sure, I'll adjust that for the next revision. The main idea here was consistency across architectures; I came across this while moving a test-case over to ARM32 and noticing a difference in the behavior.

duck-37 planned changes to this revision.Feb 11 2023, 9:44 AM
duck-37 updated this revision to Diff 496694.Feb 11 2023, 9:50 AM
duck-37 retitled this revision from [XRay] Make patchable-function-entry NOP sled implementation target-independent to [XRay] Add generic patchable-function-entry NOP sled implementation.
duck-37 edited the summary of this revision. (Show Details)

Changed some wording in the review, used a virtual function instead of hardcoded emitNops call so that X86 (and potentially other targets) can use their own implementations if necessary.