[PATCH] D22405: [PM] Add a generic 'repeat N times' pass wrapper to the new pass manager.

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 2 19:27:20 PDT 2016


silvas added a subscriber: silvas.

================
Comment at: lib/Passes/PassBuilder.cpp:278
@@ +277,3 @@
+static Optional<int> parseRepeatPassName(StringRef Name) {
+  if (!Name.consume_front("repeat<") || !Name.consume_back(">"))
+    return None;
----------------
It worries me that we're going down the path of "no proper lexer". D22724 improved the state of things somewhat.

I just have scars. Every time I've decided to not put a proper lexer or tokenization step in front of any kind of "parsing" I have always regretted it down the road.

It would be great if you could improve this in future patches (this would also probably make source location reporting much easier too since the token can store a source location).

Just an FYI. I think this is fine for now.

================
Comment at: test/Other/new-pass-manager.ll:432
@@ +431,3 @@
+; RUN:     | FileCheck %s --check-prefix=CHECK-REPEAT-LOOP-PASS
+; CHECK-REPEAT-LOOP-PASS: Starting llvm::Module pass manager run
+; CHECK-REPEAT-LOOP-PASS-NEXT: Running pass: ModuleToFunctionPassAdaptor
----------------
Please don't just blindly check the exact output. Check for what you are looking for (e.g. a particular pass being run a particular number of times).
Same for the other tests.


https://reviews.llvm.org/D22405





More information about the llvm-commits mailing list