[PATCH] D38232: [MergeICmps] Disable mergeicmps if the target does not want to handle memcmp expansion.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/Scalar/MergeICmps.cpp:46
 
-#define MERGEICMPS_DOT_ON
+// #define MERGEICMPS_DOT_ON
 
----------------
Remove rather than comment out?


================
Comment at: lib/Transforms/Scalar/MergeICmps.cpp:633
+  unsigned MaxLoadSize;
+  if(!TTI->enableMemCmpExpansion(MaxLoadSize)) {
+    return PreservedAnalyses::all();
----------------
formatting - more spacious.


================
Comment at: test/Transforms/MergeICmps/pair-int32-int32.ll:1
-; RUN: opt -mergeicmps -S -o - %s | FileCheck %s
+; RUN: opt -mergeicmps -mtriple=x86_64-unknown-unknown -S -o - %s | FileCheck %s --check-prefix=X86
+; RUN: opt -mergeicmps -S -o - %s | FileCheck %s --check-prefix=NOEXPANSION
----------------
I suspect (because I've done it a few times myself!) that these tests are going to fail on bots that don't build the x86 target. I think you'll need to create target-specific subdirectories for these tests and split them up into target-independent and target-required RUNs. 

You can probably use the test structure under tests/Transforms/SimplifyCFG as a model for how it needs to be done.


https://reviews.llvm.org/D38232