[llvm-commits] [llvm] r80428 - in /llvm/trunk: include/llvm/Target/TargetLowering.h lib/CodeGen/AsmPrinter/DwarfException.cpp lib/Target/PowerPC/PPCISelLowering.cpp lib/Target/PowerPC/PPCISelLowering.h lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86ISelLowering.h

[Bill Wendling]

On Aug 29, 2009, at 9:40 AM, Duncan Sands wrote:

> Hi Bill,
>
>> -    Asm->EmitInt8(dwarf::DW_EH_PE_pcrel | dwarf::DW_EH_PE_sdata4);
>> -    Asm->EOL("FDE Encoding (pcrel sdata4)");
>> +    Encoding = Asm->TM.getTargetLowering()- 
>> >getPreferredLSDADataFormat();
>> +    Asm->EmitInt8(Encoding);
>> +    Asm->EOL("LSDA Encoding", Encoding);
>
> here you state how you will output the data.  But I don't see the
> corresponding changes to the places where the data is actually output.
>
Eric got the LSDA to look sensible before this change. :-)  
Essentially, this is fixing a long-standing bug on Darwin where we  
weren't emitting the encoding in the CIE properly. But I'll verify  
that the LSDA looks good.

The overall reason for this patch was to make EH work for 64-bit on  
Darwin.

>> +  bool is4Byte = TD->getPointerSize() == sizeof(int32_t);
>
> Since sizeof(int32_t) is equal to 4 and always will be, why not just  
> use
> 4 here?
>
True. But I much prefer using sizeof(int32_t) to a "magic constant"  
here. It at least shows where this value is coming from.

>> +  MF = 0;
>
> Not sure what this is about?
>
I got burned by the fact that I thought MF was still valid in the  
DwarfException class when it wasn't.

-bw