From a layering perspective, it makes sense for SystemInitializerFull to live in the outermost layer, as it's the thing which makes sure liblldb pulls in all required components. Since it is only included from files in source/API (which is as it should be), maybe we could just make it a private header and move the file to source/API/SystemInitializerFull.h ?

Thanks for the update. That looks good to me.

I don't think a name like DWARFUnitDIE is a good one bacause it would make a weird is-a relationship (a DWARFDIE represetning a DW_TAG_variable is certainly not a "unit DIE" yet you could assign it to a DWARFUnitDIE&). We could have a DWARFUnitDIE type if we wanted to, but that would have to be a special type in addition to DWARFBasicDIE. However, I think that would be overkill.

Thanks for the review!

Update with fixes from code review.

Changed the code not to short circuit functions that can be overridden at run time and not to expose <func>.cfi names to the runtime.
Added a test point to check for that.

In D47275#1110065, @clayborg wrote:

Marked things that don't belong in DWARFBasicDIE.

Added support to the assembler so that it can properly parse the output generated by -fpic.

In D47136#1106470, @jyknight wrote:

Same failure, actually, without the -position-independent, too, actually, should've parsed as VK_SPARC_13. But at least we don't ever actually generate that on output.

In D46630#1110712, @asb wrote:

I think that customising the handling of MCAlignFragment is going to be the more correct (with regards to the LLVM MC design and layering) and maintainable way.

I haven't fully stepped through the ordering of function calls when producing an object. Is your concern that the current call to writeNopData from writeFragment comes too late to create the relocation?

In D46874#1110730, @jhenderson wrote:

In D46874#1109615, @grimar wrote:

What I can probably do is to compute ID based on the .text section name.

So that for no -ffunction-sections case it would emit several .stack_sizes with the same ID and so that final object would contain only a single section finally after merging them,
just like we would want.

It would work for my sample case too I think. Let me try to implement this.

Yes, I think this all makes sense. Here's a summary of what I think is best without -function-sections enabled:

// These two share a .stack_sizes section
void func1() {}
void func2() {}

// These two share a different .stack_sizes section.
void func3()  __attribute__ ((section (".text.other"))) {}
void func4()  __attribute__ ((section (".text.other"))) {}

// This has it's own .stack_sizes section in its group
template <int I> int func5() { return I; }

LGTM.

• Generate unique ID basing on begin symbol.
• Updated test case.

In D46874#1109615, @grimar wrote:

What I can probably do is to compute ID based on the .text section name.

So that for no -ffunction-sections case it would emit several .stack_sizes with the same ID and so that final object would contain only a single section finally after merging them,
just like we would want.

It would work for my sample case too I think. Let me try to implement this.

• Reimplemented LRU cache with shared_ptr and weak_ptr.

Committed as r333165.

@chandlerc: Back-from-vacation ping.
(A short response like "yes, do change X" is fine, keeps me busy :-)

Moved using std::get up, before testStructuralMatch.

Removed the non-existing registers f33, f35, and so on. I also removed the dX and qX aliases. After trying them they felt more confusing than helpful.

Does it look fine now @krasimir ?

I think that customising the handling of MCAlignFragment is going to be the more correct (with regards to the LLVM MC design and layering) and maintainable way.

LGTM, would be nice though if somebody else took a look (@klimek ?).

In D45748#1110644, @asb wrote:

The point I was trying to make is that even with these peepholes, there are trivial cases that aren't caught, such as the example I shared:

@foo = global [6 x i16] [i16 1, i16 2, i16 3, i16 4, i16 5, i16 0], align 2                                     
                                                                                                                
define i32 @main() nounwind {                                                                                   
entry:                                                                                                          
  %0 = load i16, i16* getelementptr inbounds ([6 x i16], [6 x i16]* @foo, i32 0, i32 4), align 2                
  %cmp = icmp eq i16 %0, 140                                                                                    
  br i1 %cmp, label %if.end, label %if.then                                                                     
                                                                                                                
if.then:                                                                                                        
  tail call void @abort()                                                                                       
  unreachable                                                                                                   
                                                                                                                
if.end:                                                                                                         
  ret i32 0                                                                                                     
}                                                                                                               
                                                                                                                
declare void @abort()

This generates 3 instructions for the global access when you'd obviously prefer to use two:

lui     a0, %hi(foo)
addi    a0, a0, %lo(foo)
lhu     a0, 8(a0)

I proposed one way of adding a peephole, but don't see a way of differentiating between the case above and more complex cases where the peephole is counter-productive (such as in the longer IR sample I shared). Possible the solution would be to have an IR pass perform common subexpression elimination on the getelementptr calculations - possibly such a pass already exists.

Hi alex,