[llvm] r178506 - Mips direct object exception handling regression

[Jack Carter]

Rafael,

Just to let you know I am not ignoring you.

I am digging into why we use DW_EH_PE_absptr. GAS uses DW_EH_PE_sdata4.
We also use DW_CFA_def_cfa r29 ofs 0 whereas GAS uses DW_CFA_def_cfa_register: 29.

I think the latter differences are functionally the same.

It is possible that my change fixes it as long as it isn't in a shared object, but would probably fail otherwise.

Jack
________________________________________
From: Rafael Espíndola [rafael.espindola at gmail.com]
Sent: Monday, April 01, 2013 8:50 PM
To: Jack Carter
Cc: llvm-commits
Subject: Re: [llvm] r178506 - Mips direct object exception handling regression

>  void MCObjectFileInfo::InitELFMCObjectFileInfo(Triple T) {
> -  // FIXME: Check this. Mips64el is using the base values, which is most likely
> -  // incorrect.
> -  if (T.getArch() != Triple::mips64el)
> +  if (T.getArch() != Triple::mips &&
> +      T.getArch() != Triple::mipsel &&
> +      T.getArch() != Triple::mips64 &&
> +      T.getArch() != Triple::mips64el )
>      FDECFIEncoding = dwarf::DW_EH_PE_pcrel | dwarf::DW_EH_PE_sdata4;

This means that mips is using dwarf::DW_EH_PE_absptr. That is odd to
say the least. You never replied to the code review before committing
this. Have you actually tested that this is what the gnu assembler
produces for these targets?

> +// Assembler generated object test.
> +// This tests .eh_frame descriptors minimally.

Please delete this test. It adds no value over fde-reloc.s since that
test can show what relocation is being used. A test should be very
specific about what it is testing. This one just checks the contents
of an entire section.

>
> Added: llvm/trunk/test/MC/Mips/fde-reloc.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/Mips/fde-reloc.s?rev=178506&view=auto

This is the correct test, but it is missing the little endian variants.

> +// PR15448

This is true for the x86 test, but not for this one.


Cheers,
Rafael