[PATCH] D70096: [strictfp] Replace dangling strictfp attrs with nobuiltin

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor marked 2 inline comments as done.
andrew.w.kaylor added a comment.

Thanks for the review!



================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:5307
   return materializeForwardReferencedFunctions();
 }
 
----------------
mehdi_amini wrote:
> I think you need to call the new upgrade function from here as well.
> 
OK, thanks. I wasn't clear how the materialize stuff works. Is there some different code path that would get me here? My test is passing with the code as is, so does that mean I need an additional test?


================
Comment at: llvm/test/Bitcode/compatibility-5.0.ll:1256
   call void @f.strictfp() strictfp
-  ; CHECK: call void @f.strictfp() #44
+  ; CHECK: call void @f.strictfp() #9
 
----------------
mehdi_amini wrote:
> It'd be nice to have a test that check that the callsite attribute isn't changed when used in a non-strictfp function.
In older versions of the IR, the strictfp attribute was only used on callsites (at least by clang). Does that matter? Do I need to figure out which version started adding it to function definitions? I think it was always legal since the attribute was introduced. It just had no effect until recently.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70096/new/

https://reviews.llvm.org/D70096