[RFC] CodeGenPrepare will eventually introduce dependencies to libLLVMCodeGen in libLLVMScalarOpts

Hi,

I am working on a patch for CodeGenPrepare, which introduces a use of TargetLoweringBase::InstructionOpcodeToISD (see [1] for the details).
This is usual for CodeGenPrepare to use the TargetLowering class when it is available, however, using this particular function creates linking problems.

** Context **

The TargetLowering class is part of libLLVMCodeGen, which means that in theory every consumer of libLLVMScalarOpts would have to link against libLLVMCodeGen.

In practice, so far it was not required because all the functions of TargetLowering called in CodeGenPrepare are either:

  • virtual function (resolved dynamically).
  • inlined in the header (code available statically).

However, if you use a method that does not follow this pattern (like TargetLoweringBase::InstructionOpcodeToISD), the problem will show up.

** Advices Needed **

What would be the right way of fixing that?

There are short term solutions:

  • We can inline the functions we need in the header (Hack).
  • We can mark the functions we need as virtual (Hack).
  • We can move the functions elsewhere (where?).

What would be a longer term solution: i.e., how can we prevent this problem to happen again?
The only thing I can think of is moving CodeGenPrepare elsewhere.

Thanks for your help,

-Quentin

[1] http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140217/205266.html

Hi,

I am working on a patch for CodeGenPrepare, which introduces a use of
TargetLoweringBase::InstructionOpcodeToISD (see [1] for the details).
This is usual for CodeGenPrepare to use the TargetLowering class when it
is available, however, using this particular function creates linking
problems.

** Context **

The TargetLowering class is part of libLLVMCodeGen, which means that in
theory every consumer of libLLVMScalarOpts would have to link against
libLLVMCodeGen.

In practice, so far it was not required because all the functions of
TargetLowering called in CodeGenPrepare are either:
- virtual function (resolved dynamically).
- inlined in the header (code available statically).

However, if you use a method that does not follow this pattern (like
TargetLoweringBase::InstructionOpcodeToISD), the problem will show up.

** Advices Needed **

What would be the right way of fixing that?

There are short term solutions:
- We can inline the functions we need in the header (Hack).
- We can mark the functions we need as virtual (Hack).
- We can move the functions elsewhere (where?).

None of these will work with many different linking strategies. We need to
go after the longer term solution *now* I think. We've waited long enough.

What would be a longer term solution: i.e., how can we prevent this
problem to happen again?
The only thing I can think of is moving CodeGenPrepare elsewhere.

I think there is a clear way to make this decision:

Either we want CodeGenPrepare to work *exclusively* through an abstracted
interface to the target like TargetTransformInfo like the vectorizer and
other "lowering" passes that can easily remain target independent, or we
must move CGP into lib/CodeGen.

I think the latter is the obvious answer. I have no reason to think it is
remotely plausible to route all of CGP's queries through a narrow abstract
interface like TTI. However, we already have quite a few IR-level passes in
lib/CodeGen and so sinking CGP there seems like not a bad thing. It solves
all of the horrible layering problems that are currently blocking progress
and creates no new ones I'm aware of...

From: "Quentin Colombet" <qcolombet@apple.com>
To: "LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu>
Sent: Wednesday, February 19, 2014 3:30:49 PM
Subject: [LLVMdev] [RFC] CodeGenPrepare will eventually introduce dependencies to libLLVMCodeGen in libLLVMScalarOpts

Hi,

I am working on a patch for CodeGenPrepare, which introduces a use of
TargetLoweringBase::InstructionOpcodeToISD (see [1] for the
details).
This is usual for CodeGenPrepare to use the TargetLowering class when
it is available, however, using this particular function creates
linking problems.

** Context **

The TargetLowering class is part of libLLVMCodeGen, which means that
in theory every consumer of libLLVMScalarOpts would have to link
against libLLVMCodeGen.

In practice, so far it was not required because all the functions of
TargetLowering called in CodeGenPrepare are either:
- virtual function (resolved dynamically).
- inlined in the header (code available statically).

However, if you use a method that does not follow this pattern (like
TargetLoweringBase::InstructionOpcodeToISD), the problem will show
up.

** Advices Needed **

What would be the right way of fixing that?

There are short term solutions:
- We can inline the functions we need in the header (Hack).
- We can mark the functions we need as virtual (Hack).
- We can move the functions elsewhere (where?).

What would be a longer term solution: i.e., how can we prevent this
problem to happen again?
The only thing I can think of is moving CodeGenPrepare elsewhere.

It seems to me that IR optimizations that depend directly on TargetLowering belong in CodeGen. Why don't you just move them all there?

-Hal

or we must move CGP into lib/CodeGen...[snip]

It solves all of the horrible layering problems that are currently blocking progress
and creates no new ones I'm aware of...

Seems totally sane to me.

- Lang.

Some of them *could* be profitably lifted to use a more abstract interface
like TTI. I'm pretty sure CGP is on the "not worth it" side of this
equation, and it may not be the only thing on that side, but I'm trying to
leave open the idea that there is some inflection point here and we may
have passes on both sides of it that currently are still misusing
TargetLowering and in the IR pass libraries.

Thanks all for your answers!

Seems like we agree that CodeGenPrepare should be moved into lib/CodeGen.

Following Hal’s advice to do that for every IR pass that depends directly on TargetLowering, it seems we would have to do the same thing for GlobalMerge and LowerInvoke. For the latter, I do not really know what it implies since this is part of Transforms/Utils.

Let me see what I can do with CodeGenPrepare first.

-Quentin

Well said. If we do move a bunch of things into CodeGen, should we create a subdirectory? Philip

We currently have CGP, GlobalMerge and LowerInvoke. LowerInvoke looks like it could be made to use TTI, as could GlobalMerge, because they only use a couple of methods. So I agree with you, let's just move CGP.

-Hal