[PATCH] D62766: [Attributor] Deduce "nosync" function attribute.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

The diff seems to include changes in the attributor. May download the latest version of the Attributor patch, rebase this one, and remove everything that is not part of the nosync. Also, please include the changes to the test cases you have in this patch.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:374
+
+  /// Returns true of "nosync" is assumed.
+  bool isAssumedNoSync() const { return getAssumed(); }
----------------
Typo: "if"


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:423
+                          (unsigned)Instruction::Store,
+                          (unsigned)Instruction::AtomicRMW};
+
----------------
You have to add fences here as well and look at calls explicitly (as they are not in the above opcode list). Alternatively you could do:

Use the `InformationCache::getReadOrWriteInstsForFunction` method to get all potential read & write instructions. That will include calls you need to look at and everything above. You will need to look at calls first, if they are fine, you can check for volatile and atomic. The code to determine if a call is OK is already present down there.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:431
+
+      auto ordering = getOrdering(I);
+
----------------
Please use the type here and start variables with an upper case letter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62766