[PATCH] D53608: [builtins] Build float128 soft float builtins for x86_64.

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added subscribers: MaskRay, phosek, compnerd, aaronmondal.
aaronmondal added a comment.
Herald added a subscriber: Enna1.
Herald added a project: All.

Hmm I tried to fiddle around with this but I'm out of my depth here. Pulling in 
@MaskRay @compnerd @phosek in the hopes that one of you is able to provide some 
insight or pull in someone else who is able to help.

When building a static compiler-rt with thinlto every downstream target breaks 
because of the missing `__extendxftf2` and `__trunctfxf2`. AFAIU this blocks 
everyone from using thinlto with compiler-rt.

  ld.lld: error: undefined symbol: __extendxftf2
  >>> referenced by extendsftf2.c:18 
(./external/llvm-project-overlay~17-init-bcr.0~llvm_project_overlay~llvm-project/compiler-rt/lib/builtins/extendsftf2.c:18)
  >>>   lto.tmp:(__extendsftf2)
  >>> referenced by extenddftf2.c:18 
(./external/llvm-project-overlay~17-init-bcr.0~llvm_project_overlay~llvm-project/compiler-rt/lib/builtins/extenddftf2.c:18)
  >>>   lto.tmp:(__extenddftf2)
  >>> referenced by floattitf.c:75 
(./external/llvm-project-overlay~17-init-bcr.0~llvm_project_overlay~llvm-project/compiler-rt/lib/builtins/floattitf.c:75)
  >>>   lto.tmp:(__floattitf)
  >>> referenced 1 more times
  >>> did you mean: __extenddftf2
  >>> defined in: lto.tmp
  
  ld.lld: error: undefined symbol: __trunctfxf2
  >>> referenced by trunctfsf2.c:17 
(./external/llvm-project-overlay~17-init-bcr.0~llvm_project_overlay~llvm-project/compiler-rt/lib/builtins/trunctfsf2.c:17)
  >>>   lto.tmp:(__trunctfsf2)
  >>> referenced by trunctfdf2.c:17 
(./external/llvm-project-overlay~17-init-bcr.0~llvm_project_overlay~llvm-project/compiler-rt/lib/builtins/trunctfdf2.c:17)
  >>>   lto.tmp:(__trunctfdf2)
  
bazel-out/k8-fastbuild-ST-1b2103630309/bin/external/llvm-project-overlay~17-init-bcr.0~llvm_project_overlay~llvm-project/clang/clang-linker-wrapper:
 error: 'ld.lld' failed

I also vaguely remember this leading to issues with static CUDA linking, but I 
don't have stacktraces for that :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142507: [AMDGPU] Split dot7 feature

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a comment.

I think unless conflicts arise creating an issue similar to this 
https://github.com/llvm/llvm-project/issues/60600 with the `cherry-pick` line 
set to this commit should be enough. (See also 
https://llvm.org/docs/GitHub.html).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142507

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142507: [AMDGPU] Split dot7 feature

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a comment.

> I cannot say there was much choice. The only real choice was to postpone the 
> split and magnify the problem in the future. As for the ifdefs, this might be 
> possible in the device-libs but I do not see how to do it the Builtins.def.

Hmm maybe ifdefs in the device libs would also just delay the issue. Maybe it 
really is best to pull this change into Clang 16 and accept the fact that it's 
an unfortunate situation, but at least give users with very recent hardware the 
option to use a regular Clang to build ROCm. Realistically, those actually 
upgrading to Clang 16 early will also be those upgrading to ROCm5.5 early and 
likely also be those most likely to have 7900 GPUs.

Somehow, telling users "if you have a new GPU you need new Clang + ROCm" and 
"if you want new ROCm for your old GPU you need to also upgrade Clang" sounds 
better to me than telling them "if you have a new GPU you are SOL unless you 
use binary releases or build the amd-llvm-fork" 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142507

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142507: [AMDGPU] Split dot7 feature

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a comment.

Well, I can already feel the pain that distro maintainers having to build the 
next ROCm releases 

I wonder what the better course of action is here:

1. Port this patch to Clang 16 so that users with new hardware will be able to 
build ROCm 5.5, but make it impossible to build ROCm 5.4 and older with clang 
16.
2. Don't port this patch and have a ~6 months gap during which users with the 
7900 GPUs won't be able to build ROCm with a stable Clang version, requiring 
distro maintainers to use several toolchains and source-based distro users to 
use differentl compatibility patches for different ROCm releases. So basically 
when 8900 GPUs are announced, clang would support ROCm for 7900 GPUs 

Would there be a way to retain at least *some* backwards compatibility or 
version interoperability? For instance, via an `#ifdef CLANG_VERSION_MAJOR` in 
the device libs and an `#ifdef INCOMPATIBLE_AMDGPU_INSTS` in Clang?

This would obviously very ugly, but it still seems better to me than locking 
out users (and more likely, ROCm contributors) from using 7900 GPUs if they are 
unable to build Clang themselves. Users already complain about how hard it is 
to build ROCm, and they also complain about the frequent breaking changes 
Clang. I'm very much in favor of moving fast, but I'm worried that complete 
disregard for backwards compatibility like this with no clear upgrade path or 
fallback mechanism could cause a lot of frustration for users and distro 
maintainers.

Maybe there is some other, prettier way to solve this? 凉


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142507

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142507: [AMDGPU] Split dot7 feature

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a comment.

> It shall be complimented by the device-lib change in the corresponding 
> release, so it is not that simple.

@rampitec I'm not sure I understand. Does this mean that this is breaking in a 
way that Clang 17 won't be able to build ROCm 5.4?

I thought it was like "we need D142507  to 
build device-libs after 8dc779e 
"
 and for older device libs we just fall back to some older behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142507

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142507: [AMDGPU] Split dot7 feature

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added subscribers: jhuber6, aaronmondal.
aaronmondal added a comment.

Would it be possible to backport this to Clang 16?

If 
https://github.com/RadeonOpenCompute/ROCm-Device-Libs/commit/8dc779e19cbf2ccfd3307b60f7db57cf4203a5be
 makes it into ROCm 5.5 no distro would be able to build it with "vanilla" 
Clang 16, potentially causing pain for users that try to build ROCm 5.5 with a 
Clang from a package manager (a realistic scenario, considering that one may 
want to invest 5 min to build ROCm but not 40 min to build Clang). ROCm 5.5 
will be the first release to officially support the 7900XT and 7900XTX, so not 
having this potentially causes issues for users with recent AMD hardware. (See 
https://github.com/RadeonOpenCompute/ROCm/issues/1880 for extensive, related 
discussion).

@jhuber6 This wouldn't exactly "solve" 
https://github.com/llvm/llvm-project/issues/60660, but I think this could also 
be a workaround (with potentially better user experience), as allowing users 
build ROCm with regular Clang 16 prevents that deadlock where we can't build 
ROCm anymore. This is entirely based on speculation that ROCm 5.5 won't 
introduce other breakages before its release though, so I'd totally understand 
if this is not a satisfactory solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142507

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143768: [Clang] Add options to disable direct linking of arch tools

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a subscriber: aaron.ballman.
aaronmondal added a comment.

Ok so I went over 
https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations#testing-for-the-presence-of-a-header-__has_include
 (thanks, @aaron.ballman ) and through the commit history for the amdgpu-arch 
tool.

As I understand, in cases such as standard library headers, these 
`__has_include`s are useful (and are encouraged), (I assume especially for 
header-only libraries). But I'm not convinced that it's the right way to do 
things in the HSA case.

1. If we don't detect this during configuration, we will fail during the build, 
not during configuration. this means that if the hsa CMake package is found but 
the headers are not actually present, we will fail late, potentially *very* 
late in the build.
2. It also seems like the `__has_include`s have continuously been a source of 
confusion, and maybe I'm confused as well, but I think even the current 
implementation after 
https://reviews.llvm.org/rGbfe4514add5b7ab7e1f06248983a7162d734cffb has a bug 
(if DYNAMIC_HSA is not set and hsa is not found, we don't raise a "missing 
hsa.h" error).

@jhuber6 I didn't understand what you mean with linking it into libc. But if 
the performance regression is "ignorable" I'd be in favor of just always using 
`dlopen`.

I also agree with @tianshilei1992 that if this should remain we should 
definitely have configuration for this.

IMO it would be very desirable for every out-of-llvm-tree dependency to be 
checked during configuration. If someone without access to a compile server 
runs an LLVM build that seemingly passes configuration and then fails after 90 
minutes because they had leftover cmake configs their package manager failed to 
clean up it unnecessarily inconveniences the user and we can prevent it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143768

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143768: [Clang] Add options to disable direct linking of arch tools

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a comment.

This would also allow e.g. the ROCm packages to `FORCE_ON` hsa and enjoy the 
benefits of the LLVM configuration 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143768

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143768: [Clang] Add options to disable direct linking of arch tools

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a comment.

Fun fact I now know that libxml2 also struggles with this 

The following would roughly be how zlib and others handle it:

  if (CLANG_ENABLE_HSA)
  
if (CLANG_ENABLE_HSA STREQUAL FORCE_ON)
  find_package(hsa-runtime... REQUIRED)
else()
  find_package (hsa-runtime...)
endif()
  
if (hsa-runtime_FOUND)
  # TODO: Some hsa sanity check, executed during configuration.
  # If check passes, `HAVE_HSA` is set.
  if (CLANG_ENABLE_HSA STREQUAL FORCE_ON AND NOT HAVE_HSA)
message(FATAL_ERROR "Failed to configure HSA.")
  endif()
endif()
  
set(CLANG_ENABLE_HSA "${HAVE_HSA}")
  
  else()
set(CLANG_ENABLE_HSA 0)
  endif()

Then we could remove `__have_include` and could directly check for 
`CLANG_ENABLE_HSA` during compile time.

  #ifdef CLANG_HAVE_HSA
  #include "hsa/hsa.h"
  #endif

Since HSA is used by both OpenMP and the amdgpu-arch plugin, I think it would 
make sense to not call it `CLANG_ENABLE_HSA` but instead use `LLVM_ENABLE_HSA` 
and just put it `llvm/cmake/config-ix.cmake` (where I took the above code from).

@jhuber6 This doesn't consider the "DYNAMIC_..." macros. Do you think an 
approach like this, if worked out a bit more, could work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143768

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143768: [Clang] Add options to disable direct linking of arch tools

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a comment.

Ah yeah my bad 

Hmm somehow I keep getting confused by this. I'll check how other deps handle 
this, maybe there is a better way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143768

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143768: [Clang] Add options to disable direct linking of arch tools

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a comment.

Does this address the case where we have HSA headers present on the system?

Since this only seems to modify link time behavior, I think building with this 
flag would still include "hsa/hsa.h" if they were present.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143768

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a comment.

Porting to bazel in https://reviews.llvm.org/D143344


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal requested changes to this revision.
aaronmondal added a comment.
This revision now requires changes to proceed.

I just noticed that there probably needs to be one change. The doc sometimes 
describes `-fprebuilt-module-interface`, which is not a valid option. I think 
the occurrences were meant to be `-fpbrebuilt-module-path`.


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

https://reviews.llvm.org/D131388

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal accepted this revision.
aaronmondal added a comment.

Thanks for addressing my comment. I think I overlooked the part about 
`-fmodules-embed-all-files` 

I'm fine with this revision 


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

https://reviews.llvm.org/D131388

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

aaronmondal added a comment.

@ChuanqiXu Thanks a lot for this!

We are currently implementing this module logic in the Bazel ruleset rules_ll 
.
This page helps clarify the differences between clang modules and C++ modules a 
lot. If I knew about this earlier we'd probably have saved a lot of time 

After reading this doc, a few things came to my mind. I can add a patch for 
this as a follow-up diff when I better understand how things work.

- The path logic that references the source files in BMIs is very important. 
When precompiling a module in one directory and then moving a header included 
in the global module fragment from the source file, things will break.
- As a result of this, some unintuitive things can happen when using sandboxed 
compilation. E.g. precompile BMI in sandbox, move BMI out of that sandbox, then 
start another sandbox that loads the previously built BMI. If the BMI used 
headers in a global module fragment, and those headers were in the original 
sandboxe, the header in the new sandbox will be in a different location 
(because the new sandbox is at a new location) and things will break.
- Clang modulemaps are cool, but for build systems that have to declare every 
output, implicit modulemaps seem like a bad idea. Clang's caching system, even 
with custom cache paths seems like a bad idea as well. It is probably helpful 
to mention caveats of implicit modules and how to disable them (or link to the 
Clang Module docs for this).
- The doc describes how to build e.g. an `iostream` module using the C++ 
semantics, but doesn't mention that AFAIK there is no "native" C++ Module build 
for libcxx yet. One would currently actually use Clang modules with implicit 
modulemaps. It may be helpful to mention this to avoid confusion.
- Issue https://github.com/llvm/llvm-project/issues/56770 and 
seems-to-be-a-consequence-issue 
https://github.com/llvm/llvm-project/issues/57293 may be relevant apart from 
the already mentioned issue about clang-scan-deps.


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

https://reviews.llvm.org/D131388

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123074: [clang-tidy][run-clang-tidy.py] Fix typo in new -config-file option

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

AaronSiddharthaMondal added a comment.

Ahh I didn't know that. After fixing my settings it is now linked correctly. No 
need to revert.

Thanks a lot for the help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123074

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123074: [clang-tidy][run-clang-tidy.py] Fix typo in new -config-file option

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

AaronSiddharthaMondal added a comment.

@aaron.ballman There may have been something wrong with the commit. On GitHub 
it does not link to my user account. Would it be possible to change that?
The github name is aaronmondal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123074

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123074: [clang-tidy][run-clang-tidy.py] Fix typo in new -config-file option

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

AaronSiddharthaMondal added a comment.

In D123074#3430355 , @aaron.ballman 
wrote:

> In D123074#3430351 , @aaron.ballman 
> wrote:
>
>> LGTM, thank you for the fix!
>
> If you need me to commit on your behalf, what name and email address would 
> you like me to use for patch attribution?

Name is Aaron Siddhartha Mondal. Email .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123074

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123074: [clang-tidy][run-clang-tidy.py] Fix typo in new -config-file option

[Aaron Siddhartha Mondal via Phabricator via cfe-commits]

AaronSiddharthaMondal created this revision.
AaronSiddharthaMondal added a reviewer: aaron.ballman.
AaronSiddharthaMondal added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
AaronSiddharthaMondal requested review of this revision.
Herald added a subscriber: cfe-commits.

The new `-config-file` option introduced by 9e1f4f1 
 was 
accidentally referenced
as `args.config_path` on the python side. This patch renames `args.config_path`
to `args.config_file`.

To avoid confusion with python `file` objects, the input argument for
`get_tidy_invocation` has been renamed from `config_path` to `config_file_path`.

See GitHub issue #54728  for 
a discussion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123074

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -90,7 +90,7 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config_path,
+extra_arg, extra_arg_before, quiet, config_file_path,
 config, line_filter, use_color):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
@@ -121,8 +121,8 @@
   start.append('-p=' + build_path)
   if quiet:
   start.append('-quiet')
-  if config_path:
-  start.append('--config-file=' + config_path)
+  if config_file_path:
+  start.append('--config-file=' + config_file_path)
   elif config:
   start.append('-config=' + config)
   start.append(f)
@@ -194,7 +194,7 @@
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config_path, args.config,
+ args.quiet, args.config_file, args.config,
  args.line_filter, args.use_color)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE)
@@ -304,7 +304,7 @@
  None, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config_path, args.config,
+ args.quiet, args.config_file, args.config,
  args.line_filter, args.use_color)
 invocation.append('-list-checks')
 invocation.append('-')


Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -90,7 +90,7 @@
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
 header_filter, allow_enabling_alpha_checkers,
-extra_arg, extra_arg_before, quiet, config_path,
+extra_arg, extra_arg_before, quiet, config_file_path,
 config, line_filter, use_color):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
@@ -121,8 +121,8 @@
   start.append('-p=' + build_path)
   if quiet:
   start.append('-quiet')
-  if config_path:
-  start.append('--config-file=' + config_path)
+  if config_file_path:
+  start.append('--config-file=' + config_file_path)
   elif config:
   start.append('-config=' + config)
   start.append(f)
@@ -194,7 +194,7 @@
  tmpdir, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
- args.quiet, args.config_path, args.config,
+ args.quiet, args.config_file, args.config,
  args.line_filter, args.use_color)
 
 proc = subprocess.Popen(invocation, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
@@ -304,7 +304,7 @@
  None, build_path, args.header_filter,
  args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
-