[PATCH] D63152: [FIX] Forces shrink wrapping to consider any memory access as aliasing with the stack

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

Agree, the fix is more important than the performance loss at this point.

LGTM, but comments to address below.



================
Comment at: lib/CodeGen/ShrinkWrap.cpp:267
+   *       that load and stores never derive from the stack pointer.
+   */
+  if (MI.mayLoadOrStore())
----------------
I don't think we use C style comment usually.
Could you turn that into C++ (//) ones?


================
Comment at: test/CodeGen/AArch64/pr37472.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -o - %s | FileCheck %s
+
----------------
If at all possible a MIR test would be more robust here.
Otherwise, use `opt -instnamer` to get rid of the implicit variables.

Also:
- please use a more explicit name for the filename. E.g., dont-shrink-wrap-for-stack-access.ll
- put the PR number in the comment of the test
- add a comment in the test describing the characteristic for the test: some load store that can access the stack


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63152/new/

https://reviews.llvm.org/D63152