[PATCH] D75519: [ExpandMemCmp] Properly constant-fold all compares.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D75519#1904827 <https://reviews.llvm.org/D75519#1904827>, @courbet wrote:

> In D75519#1903306 <https://reviews.llvm.org/D75519#1903306>, @efriedma wrote:
>
> > Given memcmp expansion is running late in the pipeline, do you think we're possibly missing other relevant optimizations on the loads?  LICM, maybe?
>
>
> It's definitely missing other optimizations. I tried moving it sooner in the pipeline some time ago for this reason (https://reviews.llvm.org/D60318), but it turned out to be much harder than I anticipated because of interactions with the sanitizers.


For reference, we still have at least a couple of motivating bugs for moving this up in the pipeline:
https://bugs.llvm.org/show_bug.cgi?id=36421
https://bugs.llvm.org/show_bug.cgi?id=34032#c13 - reduced example: https://godbolt.org/g/QjVXvS

I think this patch makes sense, but we might also consider a small add-on that would catch the cases shown here and possibly more. We can basically run -instsimplify within a pass via:

  diff --git a/llvm/lib/CodeGen/ExpandMemCmp.cpp b/llvm/lib/CodeGen/ExpandMemCmp.cpp
  index d0dd538f1f5..bb5e6261593 100644
  --- a/llvm/lib/CodeGen/ExpandMemCmp.cpp
  +++ b/llvm/lib/CodeGen/ExpandMemCmp.cpp
  @@ -23,6 +23,7 @@
   #include "llvm/CodeGen/TargetSubtargetInfo.h"
   #include "llvm/IR/IRBuilder.h"
   #include "llvm/InitializePasses.h"
  +#include "llvm/Transforms/Utils/Local.h"
   #include "llvm/Transforms/Utils/SizeOpts.h"
   
   using namespace llvm;
  @@ -869,6 +870,11 @@ PreservedAnalyses ExpandMemCmpPass::runImpl(
         ++BBIt;
       }
     }
  +
  +  if (MadeChanges)
  +    for (BasicBlock &BB : F)
  +      SimplifyInstructionsInBlock(&BB);
  +
     return MadeChanges ? PreservedAnalyses::none() : PreservedAnalyses::all();
   }
   


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75519