[PATCH] D93039: Introduce llvm.noalias.decl intrinsic

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

Forgot to send this nits, feel free to add in a follow up.



================
Comment at: llvm/lib/IR/Verifier.cpp:5586
+
+    // [ItCurrent, ItNext[ represents the declarations for the same scope.
+    // Ensure they are not dominating each other
----------------
s/[/]


================
Comment at: llvm/lib/IR/Verifier.cpp:5571
+
+  llvm::sort(NoAliasScopeDecls, Compare);
+
----------------
jdoerfert wrote:
> If this sorts based on pointer values it is non-deterministic. While potentially OK in the verifier, I'd recommend a TODO. We should sort by "metadata ids" (!0, !1, ...) or something that is deterministic. 
+1, please let's avoid non-deterministic behavior, follow-up works.

Potentially also store and reuse the list of scopes from the above for loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93039