status of IPO/IPCP?

Project Infrastructure LLVM Dev List Archives
1

The pass is pretty rudimental (as the comment at the top of the file
hints), and it seems LLVM already has IPSCCP (which should do a better
job at interprocedural constant propagation).
I'm also not entirely sure it's used anywhere.
Is there any reason to keep it around?

Thanks,

2

No tests fail with the patch below, so I would say it’s pretty useless. It seems that the C bindings are the only user but we can probably just have them return IPSCCP instead.

Any idea why this wasn’t removed when IPSCCP was introduced? Probably worth understanding that before ripping it out, but in the current state of things I don’t think removing it will be problematic.

diff --git a/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp b/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp
index a1533b3…24aea5c 100644
— a/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp
+++ b/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp
@@ -51,7 +51,9 @@ char IPCP::ID = 0;
INITIALIZE_PASS(IPCP, “ipconstprop”,
“Interprocedural constant propagation”, false, false)

-ModulePass *llvm::createIPConstantPropagationPass() { return new IPCP(); }
+ModulePass *llvm::createIPConstantPropagationPass() {

  • return createIPSCCPPass();
    +}

bool IPCP::runOnModule(Module &M) {
if (skipModule(M))

– Sean Silva

3

Sean Silva via llvm-dev <llvm-dev@lists.llvm.org> writes:

No tests fail with the patch below, so I would say it's pretty useless. It
seems that the C bindings are the only user but we can probably just have them
return IPSCCP instead.

I don't necessarily think your conclusion is wrong, but the patch isn't
proving what you think it's proving. In fact, the below passes all tests
as well. When you call passes through `opt` they don't end up calling
through the createXYZPass path.

diff --git a/lib/Transforms/IPO/IPConstantPropagation.cpp b/lib/Transforms/IPO/IPConstantPropagation.cpp
index b3ee499..8d70b98 100644
--- a/lib/Transforms/IPO/IPConstantPropagation.cpp
+++ b/lib/Transforms/IPO/IPConstantPropagation.cpp
@@ -253,7 +253,9 @@ char IPCP::ID = 0;
INITIALIZE_PASS(IPCP, "ipconstprop",
                 "Interprocedural constant propagation", false, false)

-ModulePass *llvm::createIPConstantPropagationPass() { return new IPCP(); }
+ModulePass *llvm::createIPConstantPropagationPass() {
+ llvm_unreachable("fnord");
+}

bool IPCP::runOnModule(Module &M) {
   if (skipModule(M))

4

I think this should do the job:

$ grep -R 'ipconstprop' * | wc -l
   7
$ sed -i.bak 's/ipconstprop/ipsccp/' *
$ grep -R 'ipconstprop' * | wc -l
       0

and two tests are actually failing :expressionless:

Testing Time: 0.10s

5

I analyzed this and realized there are some cases that are actually
not handled properly by SCCP but they are by the naive algorithm. For
example, in return-argument SCCP doesn't propagate the return value
correctly

6

I analyzed this and realized there are some cases that are actually
not handled properly by SCCP but they are by the naive algorithm. For
example, in return-argument SCCP doesn't propagate the return value
correctly, so the result is

  %Z = add i32 X1, X2

instead of

  %Z = add i32 1, 3

I suspect this is because IPSCCP doesn't (yet) handle multiple ret
value instructions.

7

Sean Silva via llvm-dev <llvm-dev@lists.llvm.org> writes:
> No tests fail with the patch below, so I would say it's pretty useless.
It
> seems that the C bindings are the only user but we can probably just
have them
> return IPSCCP instead.

I don't necessarily think your conclusion is wrong, but the patch isn't
proving what you think it's proving. In fact, the below passes all tests
as well. When you call passes through `opt` they don't end up calling
through the createXYZPass path.

Ah, interesting! Thanks for catching this!

-- Sean Silva