Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Redesign ConstantExpr's #10740

Open
lattner opened this issue Jul 15, 2011 · 4 comments
Open

Redesign ConstantExpr's #10740

lattner opened this issue Jul 15, 2011 · 4 comments
Labels
bugzilla Issues migrated from bugzilla code-cleanup llvm:core

Comments

@lattner
Copy link
Collaborator

lattner commented Jul 15, 2011

Bugzilla Link 10368
Version 1.0
OS All
CC @asl,@zmodem,@pcc,@rengolin,@rjmccall

Extended Description

We currently allow ConstantExprs occur for any side-effect free LLVM IR instruction, and use ConstantExpr::get as the primitive constant folding API in LLVM.

This doesn't make sense for a number of reasons:

a) code generators can't codegen arbitrary constant exprs at arbitrary places, causing global variable initializers to explode in funny ways if the optimizer gets creative.
b) constant expressions like divides canTrap() which is a common source of bugs.
c) optimizers typically treat constants as cheap or free, but a ConstantExpr can have arbitrary cost.
d) optimizers try to handle both instructions and constantexprs equally well in many cases, leading to complexity in PatternMatch.h and to the existence of the Operator classes, which are gross.
e) we have no way to represent target-specific relocations that don't conveniently map to ConstantExprs.

Instead, I propose that we implement the following plan:

  1. Switch everything off using ConstantExpr::get as constant folding API, onto the InstSimplify APIs, which are more general and powerful anyway.
  2. Move all of the constant folding logic out of ConstantExpr::get into InstSimplify so that ConstantExpr::get just creates them.
  3. Remove the canTrap() constant exprs, simplifying the world and cleaning up tons of stuff.
  4. Introduce new ConstantExprs for target specific things that we currently need like darwin_symbol_difference.
  5. Remove various extraneous constantexprs other than GEP and bitcast.
  6. Introduce a new ConstantExpr "base_offset" node which takes a constant pointer and integer and adds them together, returning a new pointer. Teach the optimizer to optimize them as well as ConstantExpr GEP's are right now.
  7. Remove the gep constantexpr, leaving us with the target specific ones, bitcast, and base_offset.
  8. Profit!

-Chris
@rengolin
Copy link
Member

Chris,

Reading again the description, it seems steps 1 and 2 are the same thing. I'm assuming you meant InstCombine, which has a lot of visitors, not all of them pertinent to constant folding.

Maybe there is another pass that does it to constants, or there could be a visitor on InstCombine that calls all constant folding visitors, so we could call that pass at the same time (if possible) as the IR being built, emulating the current behaviour, and breaking less tests for now.

The new visitors (the ones created by moving ::get folding to InstCombine) can also run on other constants, during other passes, de-duplicating the logic and making the move easier.

Later on, we can add more foldings and remove the need for folding while building IR independently of each other, and fixing the tests.

Does it make sense?

@lattner
Copy link
Collaborator Author

lattner commented Jul 28, 2012

What I was getting at is that things-other-than-instcombine need an API to do foldings in various other contexts. Today, clients use the instsimplify API to do this, but some still use ConstantExpr::get. We should switch everyone off ConstantExpr::get onto instsimplify. InstCombine can then do more interesting and higher level work based on dominator information and other things it has access to.

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 3, 2014

It is a hack, but somewhat in line with what we did for GlobalAlias. Instead of knowing at the llvm level if an expression is valid or not, just lower it and let MC decide.

We would print things like

call (foo-bar)/(zed-bah).....

and let MC figure out if there is a relocation that can be used. This is quiet fuzzy as passes then have to be conservative about which expressions they are allowed to build, but they already have to do it for GlobalAlias and it would avoid what seems to be the main point point of the current implementation:

  • Constants can have arbitrary cost.
  • Constants can trap.

@pcc
Copy link
Contributor

pcc commented Aug 26, 2015

Related proposal:

http://lists.llvm.org/pipermail/llvm-dev/2015-July/088748.html

Further discussion:

http://lists.llvm.org/pipermail/llvm-dev/2015-August/089693.html

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bugzilla Issues migrated from bugzilla code-cleanup llvm:core
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@pcc @rengolin @lattner @llvmbot