[PATCH] D48832: [ARM] ARMCodeGenPrepare backend pass

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

Also a quick first pass, see comments inline.



================
Comment at: lib/Target/ARM/ARMCodeGenPrepare.cpp:49
+static cl::opt<bool>
+DisableDSP("arm-disable-scalar-dsp", cl::Hidden, cl::init(false),
+          cl::desc("Don't use DSP instructions for scalar operations"));
----------------
Can you comment what this exactly does? Looks like that deep down in the logic that checks for candidares, in ARMCodeGenPrepare::isNarrowInstSupported(Instruction *I), this prevents the transformation from triggering. So checking DisableDSP can be done a lot earlier, or perhaps you want to put a TODO somewhere if you plan on supporting this later? Also, this one does not seem to be covered by tests (arm-disable-scalar-dsp-imms is covered).


================
Comment at: lib/Target/ARM/ARMCodeGenPrepare.cpp:184
+
+  if (isa<StoreInst>(V) || isa<TerminatorInst>(V))
+    return false;
----------------
I learned a trick the other day: the IR pattern matcher. ;-) Looks like you can simply things, and don't need the check and cast above and can immediately do something like:

   if (match(V, m_Store(m_Value(), m_Value())) || isa<TerminatorInst>(V))
     return false;

Looks like you can actually use the pattern matcher at more places, like e.g. the function below.


https://reviews.llvm.org/D48832