Skip to content

[SelectionDAG] Make the FoldingSet profile in getAtomic match AddNodeIDCustom. #136651

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 22, 2025

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Apr 22, 2025

The code in AddNodeIDCustom:

const AtomicSDNode *AT = cast(N);
ID.AddInteger(AT->getMemoryVT().getRawBits());
ID.AddInteger(AT->getRawSubclassData());
ID.AddInteger(AT->getPointerInfo().getAddrSpace()); ID.AddInteger(AT->getMemOperand()->getFlags());

In theory, the mismatch would have made CSE of AtomicSDNodes not work, but I don't know how to test it.

…IDCustom.

The code in AddNodeIDCustom:

const AtomicSDNode *AT = cast<AtomicSDNode>(N);
ID.AddInteger(AT->getMemoryVT().getRawBits());
ID.AddInteger(AT->getRawSubclassData());
ID.AddInteger(AT->getPointerInfo().getAddrSpace());
ID.AddInteger(AT->getMemOperand()->getFlags());

In theory, the mismatch would have made CSE of AtomicSDNodes not
work, but I don't know how to test it.
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Apr 22, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 22, 2025

@llvm/pr-subscribers-llvm-selectiondag

Author: Craig Topper (topperc)

Changes

The code in AddNodeIDCustom:

const AtomicSDNode *AT = cast<AtomicSDNode>(N);
ID.AddInteger(AT->getMemoryVT().getRawBits());
ID.AddInteger(AT->getRawSubclassData());
ID.AddInteger(AT->getPointerInfo().getAddrSpace()); ID.AddInteger(AT->getMemOperand()->getFlags());

In theory, the mismatch would have made CSE of AtomicSDNodes not work, but I don't know how to test it.


Full diff: https://github.com/llvm/llvm-project/pull/136651.diff

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+3-1)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index ba6c5d884d381..5269962ea2062 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -8994,8 +8994,10 @@ SDValue SelectionDAG::getAtomic(unsigned Opcode, const SDLoc &dl, EVT MemVT,
                                 SDVTList VTList, ArrayRef<SDValue> Ops,
                                 MachineMemOperand *MMO) {
   FoldingSetNodeID ID;
-  ID.AddInteger(MemVT.getRawBits());
   AddNodeIDNode(ID, Opcode, VTList, Ops);
+  ID.AddInteger(MemVT.getRawBits());
+  ID.AddInteger(getSyntheticNodeSubclassData<AtomicSDNode>(
+      Opcode, dl.getIROrder(), VTList, MemVT, MMO));
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   ID.AddInteger(MMO->getFlags());
   void* IP = nullptr;

Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

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

LGTM

@topperc topperc merged commit 497382e into llvm:main Apr 22, 2025
13 checks passed
@topperc topperc deleted the craigt/atomic-profile branch April 22, 2025 05:39
@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 22, 2025

LLVM Buildbot has detected a new failure on builder lldb-remote-linux-ubuntu running on as-builder-9 while building llvm at step 16 "test-check-lldb-api".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/7936

Here is the relevant piece of the build log for the reference
Step 16 (test-check-lldb-api) failure: Test just built components: check-lldb-api completed (failure)
...
PASS: lldb-api :: types/TestCharTypeExpr.py (1246 of 1255)
PASS: lldb-api :: python_api/watchpoint/watchlocation/TestTargetWatchAddress.py (1247 of 1255)
PASS: lldb-api :: types/TestIntegerType.py (1248 of 1255)
PASS: lldb-api :: types/TestRecursiveTypes.py (1249 of 1255)
PASS: lldb-api :: types/TestIntegerTypeExpr.py (1250 of 1255)
PASS: lldb-api :: types/TestShortType.py (1251 of 1255)
PASS: lldb-api :: types/TestLongTypes.py (1252 of 1255)
PASS: lldb-api :: types/TestShortTypeExpr.py (1253 of 1255)
PASS: lldb-api :: types/TestLongTypesExpr.py (1254 of 1255)
TIMEOUT: lldb-api :: python_api/process/cancel_attach/TestCancelAttach.py (1255 of 1255)
******************** TEST 'lldb-api :: python_api/process/cancel_attach/TestCancelAttach.py' FAILED ********************
Script:
--
/usr/bin/python3.12 /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --libcxx-include-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/c++/v1 --libcxx-include-target-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/aarch64-unknown-linux-gnu/c++/v1 --libcxx-library-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib/aarch64-unknown-linux-gnu --arch aarch64 --build-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/lldb --compiler /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang --dsymutil /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --lldb-obj-root /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/tools/lldb --lldb-libs-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --platform-url connect://jetson-agx-2198.lab.llvm.org:1234 --platform-working-dir /home/ubuntu/lldb-tests --sysroot /mnt/fs/jetson-agx-ubuntu --env ARCH_CFLAGS=-mcpu=cortex-a78 --platform-name remote-linux --skip-category=lldb-server /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/python_api/process/cancel_attach -p TestCancelAttach.py
--
Exit Code: -9
Timeout: Reached timeout of 600 seconds

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 497382ee07100f3698621fc48b66a0bd50a1ca2a)
  clang revision 497382ee07100f3698621fc48b66a0bd50a1ca2a
  llvm revision 497382ee07100f3698621fc48b66a0bd50a1ca2a

--
Command Output (stderr):
--
WARNING:root:Custom libc++ is not supported for remote runs: ignoring --libcxx arguments
FAIL: LLDB (/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang-aarch64) :: test_scripted_implementation (TestCancelAttach.AttachCancelTestCase.test_scripted_implementation)

--

********************
Slowest Tests:
--------------------------------------------------------------------------
600.04s: lldb-api :: python_api/process/cancel_attach/TestCancelAttach.py
180.96s: lldb-api :: commands/command/script_alias/TestCommandScriptAlias.py
70.56s: lldb-api :: commands/process/attach/TestProcessAttach.py
40.04s: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py
35.42s: lldb-api :: functionalities/completion/TestCompletion.py
34.28s: lldb-api :: functionalities/single-thread-step/TestSingleThreadStepTimeout.py
25.03s: lldb-api :: python_api/watchpoint/watchlocation/TestTargetWatchAddress.py
20.93s: lldb-api :: commands/statistics/basic/TestStats.py
20.70s: lldb-api :: functionalities/gdb_remote_client/TestPlatformClient.py
19.49s: lldb-api :: functionalities/thread/state/TestThreadStates.py
18.34s: lldb-api :: commands/dwim-print/TestDWIMPrint.py
14.81s: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
14.56s: lldb-api :: commands/expression/expr-in-syscall/TestExpressionInSyscall.py
14.35s: lldb-api :: python_api/find_in_memory/TestFindRangesInMemory.py

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…IDCustom. (llvm#136651)

In theory, the mismatch would have made CSE of AtomicSDNodes not work,
but I don't know how to test it.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…IDCustom. (llvm#136651)

In theory, the mismatch would have made CSE of AtomicSDNodes not work,
but I don't know how to test it.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…IDCustom. (llvm#136651)

In theory, the mismatch would have made CSE of AtomicSDNodes not work,
but I don't know how to test it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants