Is there a reason why MCAsmStreamer class doesn't have its own .h file?

Project Infrastructure LLVM Dev List Archives
1

Does anybody know if there is a particular reason why MCAsmStreamer doesn’t have its own .h file?

https://github.com/llvm-mirror/llvm/blob/0e66a5f53c74056f95d178c86531d7d9cfb23da9/lib/MC/MCAsmStreamer.cpp#L41

It seems like it is a good idea to have this class declared as its own module ( its own .cpp and .h files). That would make it easier to inherit from it if there is a need (like in my current case).

Any help is appreciated.

2

Isn’t it also marked ‘final’ so it can’t be inherited from anyway? What’s your need to inherit from it?

3

Oops, missed the final part. I need to change the alignment. For my target
it is sort of independent of the data layout. I was going to overwrite
void MCAsmStreamer::EmitValueToAlignment(unsigned ByteAlignment, int64_t
Value, unsigned ValueSize, unsigned MaxBytesToEmit).

What is the reason it was made final? Why not to separate this class into
its own module?

4

I dont’ know why its final. That routine is just a method to force the current write pointer to a specific alignment. Shouldn’t you be changing the callers to give you the alignment you want or don’t want.

5

It looks like the callers get their information from the data layout
object, which in my case is not what I need. Any suggestions?

6

At least one of the callers would be from the .align directive which I assume you wouldn’t want to overload. Where does it come from DataLayout? AsmStreamer is used for assembling from text and I wouldn’t expect that to use DataLayout. Or is it used for inline assembly?

7

I have patches to move parts of it to a header:

http://reviews.llvm.org/D11822
http://reviews.llvm.org/D11823

8

Hi Craig and Rail,

At Movidius, we have had to make a few changes to ‘MCAsmStreamer’ to support our assembler which is not ‘gas’ compliant.

Earlier versions of LLVM (3.1 and 3.2) did have a separate header for ‘MCAsmStreamer’, and we had previously sub-classed this.

The following are modifications that we have had to make because although ‘MCAsmStreamer’ does most of what we need, there are some things that we need to do differently, and if this was available to be sub-classed, then these modifications would be more logical and not require altering the target independent code.

· ‘MCAsmStream::EmitAssignment’ has been changed to accommodate our different syntax for ‘.alias’ directives.

· ‘MCAsmStream::EmitBytes’ has been changed so that multiple lines of ‘.byte’ directives are emitted to avoid a line-length buffer limitation in our assembler when very large numbers of bytes are to be emitted.

· ‘MCAsmStream::EmitFill’ has been changed to accommodate our different syntax for ‘.fill’ directives.

These are quite simple changes, and since ‘MCAsmStreamer’ already does everything else the way we need it, it does not make sense for us to have a completely new alternative implementation of ‘MCStreamer’ which effectively clones what this class already does (and it would make it harder for us to mirror changes between releases). All of these methods are already polymorphic, so overriding the implementation in ‘MCAsmStreamer’ would be the best solution for our compiler.

Thanks,

MartinO

9

It appears that the intended mechanism for extending the AsmStreamer is through the MCTargetStreamer interface:
http://llvm.org/docs/doxygen/html/classllvm_1_1MCTargetStreamer.html

Obviously, that may not be enough flexibility for certain out of tree targets. EmitBytes is pretty similar for all in-tree targets currently. If you can make your target’s assembly work through that interface, it’ll probably be more stable going forwards.

10

At least one of the callers would be from the .align directive which I
assume you wouldn't want to overload. Where does it come from DataLayout?
AsmStreamer is used for assembling from text and I wouldn't expect that to
use DataLayout.

I made that assumption based on a very cursory look through the code.
Didn't have the time to study it thoroughly. Sorry if this created any
confusion.

Or is it used for inline assembly?

11

If you know how to does LLVM determine the alignment for a given processor
I'd appreciate if you can point it out. I've been at it since last night
and still can't figure it out.

12

I think we are talking about two different things here. There is
MCAsmStreamer,
LLVM: lib/MC/MCAsmStreamer.cpp Source File, line 41,
as well as MCTargetStreamer,
http://llvm.org/docs/doxygen/html/classllvm_1_1MCTargetStreamer.html. One
can't implement an interface (i.e overwrite a method) for MCAsmStreamer
since it is marked as final.

13

Hi Craig,

I know I'm a bit late to the party, but I wanted to chime in and say that in my past two backends we've had to copy/paste MCAsmStreamer into our target just to override one or two methods.

For instance, in our current project we need to override:

EmitLabel() - We must escape certain characters
EmitBytes() - We must also escape and disallow certain characters, and fall back to emitting byte arrays
EmitLocalCommonSymbol() - We don't want the hard-coded "lcomm" symbol
EmitAssignment() - We don't use the hard-coded syntax

There will probably be more things we need to change further down the line, too.

I believe in the last project we had to override EmitLabel() because it used label prefix instead of a label suffix.

Some of these things might be fixable using the current architecture (like putting "lcomm" into MCAsmInfo) or by moving more into MCTargetStreamer and allowing that to do the entire job, instead of only part of it. For instance, I see MCAsmStreamer::EmitLabel calls MCStreamer::EmitLabel which calls MCTargetStreamer::EmitLabel, but then after all that happens, the MCAsmStreamer implementation still unconditionally does its own logic.

That, or it could just allow targets to inherit from it as they like. Is there a reason why it couldn't or shouldn't be done?

Thanks,
Fraser