Page MenuHomePhabricator

[AArch64] Make sure XRay pseudo-instruction sizes are reported correctly
Changes PlannedPublic

Authored by duck-37 on Apr 29 2022, 5:44 PM.

Details

Reviewers
None
Summary

AArch64InstrInfo::getInstSizeInBytes does not take XRay instrumentation opcodes into account.
The default case of this function unconditionally returns 4, except for size definitions specified by TableGen.
However, on AArch64, the XRay pseudo-instructions expand to 32 bytes.
Since the XRay pseudo-opcodes are target-independent, the TableGen path doesn't get hit and therefore the size returned is wrong.

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

Diff Detail

Unit TestsFailed

TimeTest
300 msx64 debian > Clangd Unit Tests._/ClangdTests/DiagnosticTest::ClangTidyEnablesClangWarning
Script: -- /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/tools/extra/clangd/unittests/./ClangdTests --gtest_filter=DiagnosticTest.ClangTidyEnablesClangWarning
310 msx64 debian > LLVM.CodeGen/AArch64::machine-outliner-patchable.ll
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/machine-outliner-patchable.ll -verify-machineinstrs -enable-machine-outliner | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/AArch64/machine-outliner-patchable.ll

Event Timeline

duck-37 created this revision.Apr 29 2022, 5:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2022, 5:44 PM
duck-37 requested review of this revision.Apr 29 2022, 5:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2022, 5:44 PM
duck-37 updated this revision to Diff 426200.Apr 29 2022, 5:54 PM

Add newline to end of test case

duck-37 planned changes to this revision.EditedApr 29 2022, 7:05 PM

Okay, taking this off the review queue until I take a look at the other bug incidentally exposed by fixing this one; the MachineOutliner doesn't properly adjust attributes of new outlined functions with XRay instrumentation opcodes. (should it even run on those at all?)
As such, what happens is that this check doesn't pass, meaning CurrentFnBegin isn't set, meaning that an assertion failure is thrown when the code generator tries to use it as a symbol when emitting the XRay instrumentation table.
The reason this testcase didn't fail before is because AArch64InstrInfo::getOutliningCandidateInfo uses the sizes of instructions as part of the outlining heuristic. When the pseudo-opcode sizes were fixed, the heuristic decided it was profitable to outline an XRay pseudo-instruction, triggering the bug.

This comment was removed by duck-37.