r177329 - This code works around what appears to be a bug in another part of clang.

reed kotler rkotler at mips.com
Mon Mar 18 16:29:08 PDT 2013


On 03/18/2013 03:42 PM, Rafael EspĂ­ndola wrote:
>> This code works around what appears to be a bug in another part of clang.
>> I have filed http://llvm.org/bugs/show_bug.cgi?id=15538 against clang.
>> This code is safer anyway because "cast" assumes you really know that
>> it's okay to make the cast. In this case isa should not be false and
>> dyn_cast should not return null as far as I understand. But everything
>> else is valid so I did not want to revert my previous patch for attributes
>> mips16/nomips16 or use an llvm_unreachable here which would make a number
>> of our tests fail for mips.
> I don't think this is a reasonable change. If there is a bug somewhere
> else, it should be fixed. This change just hides it. It doesn't even
> include a testcase. Please revert. If some other patch caused problem,
> revert it too.
>
> Cheers,
> Rafael
Hi Rafael,

I don't think I should have to revert this patch.

I'm not even sure if there is another bug.

I don't know the code that is calling this. It's just my opinion that 
there is some other issue.

The other code is this .cpp is doing the same casting that I was doing; 
that is where I got the code from. Originally I copied exactly what the 
x86 was doing but on checkin was told to change it to it's current form 
by Bill Wendling. This current form already appears exactly as my code 
does for some other targets so they have the same problem potentially.

Everything works for mips now.

Someone that knows clang can explain how FD is a function declaration 
but GV is not a Function.
Why is that my responsibility to sort that out?

                             CodeGen::CodeGenModule &CGM) const {
      const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
      if (!FD) return;
-    llvm::Function *Fn = cast<llvm::Function>(GV);
+    llvm::Function *Fn = dyn_cast<llvm::Function>(GV);
+    if (!Fn) return; // should not happen
      if (FD->hasAttr<Mips16Attr>()) {
        Fn->addFnAttr("mips16");

This code is need for the mips test-suite to not regress and for my attribute work to continue.

Reed


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130318/7c32c04b/attachment.html>


More information about the cfe-commits mailing list