Page MenuHomePhabricator

[SelectionDAG] Properly copy ExtraInfo on RAUW
AcceptedPublic

Authored by melver on Mon, Aug 1, 1:57 AM.

Details

Summary

During SelectionDAG legalization SDNodes with associated extra info may
be replaced with a new SDNode. Preserve associated extra info on
ReplaceAllUsesWith and remove entries in DeallocateNode.

Depends on D130880

Diff Detail

Unit TestsFailed

TimeTest
60,120 msx64 debian > AddressSanitizer-x86_64-linux.TestCases::scariness_score_test.cpp
Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only -m64 -O0 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/asan/TestCases/scariness_score_test.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/asan/X86_64LinuxConfig/TestCases/Output/scariness_score_test.cpp.tmp

Event Timeline

melver created this revision.Mon, Aug 1, 1:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Aug 1, 1:57 AM
melver requested review of this revision.Mon, Aug 1, 1:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Aug 1, 1:57 AM
melver updated this revision to Diff 450026.Thu, Aug 4, 9:57 AM

Rebase.

Would you be able to reproduce the issue as a new test in TEST_F(SelectionDAGAddressAnalysisTest ?

melver updated this revision to Diff 451407.Wed, Aug 10, 3:17 AM

Add unit test for RAUW.
Note: Since SelectionDAG construction needs a target, the AArch64 test is used here (as done for several other test cases that are arch-independent).

Would you be able to reproduce the issue as a new test in TEST_F(SelectionDAGAddressAnalysisTest ?

That test seems only related to BaseIndexOffset::computeAliasing(). Added to a different test that seems to be used for that (needs a target, so there's an AArch64 one).

vitalybuka accepted this revision.Wed, Aug 10, 11:05 AM
vitalybuka added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
11819

this assert is inconsistent with the rest of the code

This revision is now accepted and ready to land.Wed, Aug 10, 11:05 AM
melver updated this revision to Diff 451648.Wed, Aug 10, 3:11 PM
  • Assert message.
melver marked an inline comment as done.Wed, Aug 10, 3:14 PM
melver added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
11819

It's similar to the one in transferDbgValues(). Only that copyExtraInfo already takes the unwrapped pointers as it makes usage a bit easier where we don't have the source SDValue. I'd like to keep it to prevent easily avoidable mistakes.