[PATCH] D123198: [LibCalls] Add argument extension attributes to more functions.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added a comment.

> I dont stiĺ think this is a correct place.
>
> If this attribute is required for correctness, you should write simple pass which adds this atrribute very soon in pipeline and runs even under -O0. Or @efriedma may propose something…

It is not a question if it is required for correctness or not (even though not all targets have this in their ABIs). The SystemZ Clang Front End does this already, so there should not be any need for an early pass to do this. Basically all front ends emitting code for SystemZ are obliged to annotate call parameters with the proper extension attribute, but that is not what this patch is handling.

> This is by design - if -ifno-builtin-xxx is used, a libcall is not annotated with any attribute here. This works fine and you are misusing BuildLibCalls and the solution is not to change when and how BuildLibCalls runs.

I would hope that with -fno-builtin the original call would be already be correctly emitted by Clang (if not, that's the place to fix it probably).

I don't understand your objections to this patch. I am not changing when or how BuildLibCalls is used, merely making sure it adds the needed parameter attributes for the targets that need it. If your concern is about chown() and lchown(), let's see what Eli has to say about that...


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

https://reviews.llvm.org/D123198