[PATCH] D112942: target ABI: improve call parameters extensions handling

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added a comment.

> Unless you're using some attribute to modify the calling convention, everything in clang defaults to "C". Some targets, like x86 Windows, frequently use alternate calling conventions, but I assume SystemZ doesn't.

I still feel more comfortable with the "external / internal" heuristic, as the whole idea is that we actually are using a trick by assuming we will be safe in clang by never acting on this optimization. For me this attribute is more readable as it reflects the idea directly, compared to checking for "C". I also feel that it is very precise and can therefore never go wrong.

> I think I'm okay leaving it in target-specific code; unifying anything related to calls is hard.

Ok, makes sense.

SPEC builds fine now after the handling of library calls arguments extensions we did earlier, but now have encountered problems when building the llvm tests: AddressSanitizer seems to be dropping these attributes:

Compiling gtest-all (reduced):

  extern "C" void *memset(void *, int, long);
  memset(&hints, 0, sizeof(hints));    
  
  =>
  
  declare ptr @memset(ptr noundef, i32 noundef signext, i64 noundef)
  %call = call ptr @memset(ptr noundef %hints, i32 noundef signext 0, i64 noundef 16)
  
  =>  InstCombine
  
  declare void @llvm.memset.p0.i64(ptr nocapture writeonly, i8, i64, i1 immarg)
  call void @llvm.memset.p0.i64(ptr noundef nonnull align 8 dereferenceable(16) %hints, i8 noundef signext 0, i64 nound
  ef 16, i1 false)
  
  =>  AddressSanitizer
  
  declare ptr @__asan_memset(ptr, i32, i64)
  %24 = call ptr @__asan_memset(ptr %12, i32 0, i64 16)

This last step is replacing an intrinsic with a call to a global function. I believe there may be some work in progress on how these attributes are supposed to be eventually managed (only in declaration, only on call instruction, either, or both), and would like to first ask how I should go about this: InstCombiner is transferring the attribute when it's building the replacing call, and it seems that AddressSanitizer should do the same. Or is there another preferred approach?


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

https://reviews.llvm.org/D112942