nelhage / llama

Apache License 2.0
589 stars 24 forks source link

Forward preprocessor definitions to the remote compiler. #42

Open jpeach opened 3 years ago

jpeach commented 3 years ago

When doing partial local preprocessing, a remote clang compilation still needs the preprocessor definitions from the command line so that the remaining preprocessing can be performed correctly. Partial preprocessing is necessary to prevent the remote compiler issuing spurious warnings from macro expansion that would be suppressed in a local compilation.

This means when spawning a compilation, we should always use either LocalArgs or RemoteArgs, and the need to separately preserve unknown arguments and conditionally restore preprocessor definitions goes away.

This updates #37.

jpeach commented 3 years ago

I haven't tested this with gcc, so it's possible that I've missed some compatibility implications, but it seems to me that it should always be safe to forward preprocessor definitions to the remove compiler. Worst case, they would just get ignored. :crossed_fingers:

jpeach commented 3 years ago

I haven't tested this with gcc, so it's possible that I've missed some compatibility implications, but it seems to me that it should always be safe to forward preprocessor definitions to the remove compiler. Worst case, they would just get ignored.

I think it's possible that this could break builds with -Wunused-command-line-argument

nelhage commented 3 years ago

Hm, this is breaking remote preprocessing because including all the arguments when detecting dependencies is problematic. e.g. it's very important that we strip any existing -M* arguments when we construct our own -M command-line.

It also OOMs if I run it at wide parallelism -- and main does not -- so I think it might be the cause of your memory issues in some way I don't fully understand yet.

nelhage commented 3 years ago

(When I get more time and energy to focus on Llama, it seems pretty clear to me that the next project is an integration test setup that runs the runtime inside a docker image and mocks out the Lambda invoke interface so we can actually test these things end-to-end in CI; there are just too many permutations to keep track of…)

jpeach commented 3 years ago

Hm, this is breaking remote preprocessing because including all the arguments when detecting dependencies is problematic. e.g. it's very important that we strip any existing -M* arguments when we construct our own -M command-line.

Oh, my intention here was to forward -U and -D. I must have been a bit naive :)

jpeach commented 3 years ago

Hm, this is breaking remote preprocessing because including all the arguments when detecting dependencies is problematic. e.g. it's very important that we strip any existing -M* arguments when we construct our own -M command-line.

AFAICT -M flags are never in Defs() and always get stripped from the remote. There is a case in the args test that covers this.

It also OOMs if I run it at wide parallelism -- and main does not -- so I think it might be the cause of your memory issues in some way I don't fully understand yet.

I added some info to #45. I think this is caused by transferring large preprocessed blobs over the net/rpc protocol, in which case I'd expect to see it whenever preprocessing is local.