[XLA] When LLVM doesn't know a CC that is more recent, warn only to developers. #41936

nouiz · 2020-07-31T13:08:20Z

The end user can't do anything about it and this confuse them as they think they have an not-optimal software stack that they should fix.

@sanjoy @thomasjoerg

thomasjoerg · 2020-07-31T15:00:43Z

tensorflow/compiler/xla/service/gpu/llvm_gpu_backend/gpu_backend_lib.cc

@@ -93,7 +93,11 @@ static string GetSmName(std::pair<int, int> compute_capability) {
    }
  }

-  if (sm_version != compute_capability_version) {
+  // If the current CC isn't supported by LLVM and it is newer then
+  // the max supported LLVM version, do not warn about it. The end


Can we please add that "Code compiled for SM75 will run on SM80 too".

+1, though I'd s/Code/PTX/

sanjoy · 2020-07-31T17:19:28Z

tensorflow/compiler/xla/service/gpu/llvm_gpu_backend/gpu_backend_lib.cc

@@ -93,7 +93,11 @@ static string GetSmName(std::pair<int, int> compute_capability) {
    }
  }

-  if (sm_version != compute_capability_version) {
+  // If the current CC isn't supported by LLVM and it is newer then
+  // the max supported LLVM version, do not warn about it. The end


+1, though I'd s/Code/PTX/

sanjoy · 2020-07-31T17:23:36Z

tensorflow/compiler/xla/service/gpu/llvm_gpu_backend/gpu_backend_lib.cc

+  // the max supported LLVM version, do not warn about it. The end
+  // user can't do anything about this.
+  if (sm_version != compute_capability_version &&
+      compute_capability_version < *(supported_version.begin())) {


This could just be lack of coffee, but where is supported_version defined?

nouiz · 2020-07-31T18:36:03Z

I amended the commit. It is true that I didn't take as much coffee today then usually :) I'll get one more now!

sanjoy

I amended the commit. It is true that I didn't take as much coffee today then usually :) I'll get one more now!

Btw, I meant maybe I need to get more coffee since I thought I was missing something obvious. :D

sanjoy · 2020-07-31T20:09:12Z

tensorflow/compiler/xla/service/gpu/llvm_gpu_backend/gpu_backend_lib.cc

@@ -86,14 +86,20 @@ static string GetSmName(std::pair<int, int> compute_capability) {
  int sm_version = 35;
  // If the current compute capability isn't known, fallback to the
  // most recent version before it.
-  for (int v : {75, 72, 70, 62, 61, 60, 53, 52, 50, 37, 35}) {
+  auto supported_version = {75, 72, 70, 62, 61, 60, 53, 52, 50, 37, 35};


Does this create a dangling reference? (I can never keep all of the C++ rules in my head.)

That variable is inside a scope and doesn't leak to anywhere. So why do you think it could be a dangling reference?

I was under the impression that the {a, b, c} syntax creates an std::initializer_list and that is only alive for the duration of the {75, 72, 70, 62, 61, 60, 53, 52, 50, 37, 35}; statement. Confusingly this seems to also have changed in C++14. https://en.cppreference.com/w/cpp/utility/initializer_list

CC @timshen91 in case he has guidance here.

gbaned · 2020-08-03T17:26:05Z

@nouiz Can you please resolve conflicts? Thanks!

trentlo · 2020-08-04T01:56:29Z

@nouiz Can you please resolve conflicts? Thanks!

Just input that Frederic is currently on vacation. Let me know if there is any urgent issues and I will help to deal with them; otherwise let's wait for his return.

nouiz · 2020-08-17T14:51:17Z

I updated this PR.

tensorflow/compiler/xla/service/gpu/llvm_gpu_backend/gpu_backend_lib.cc

…t. The end user can't do anything about it.

nouiz · 2020-08-19T14:28:33Z

I amended the commit for both comments.

sanjoy

lgtm

google-ml-butler bot added the size:S CL Change Size: Small label Jul 31, 2020

google-ml-butler bot requested a review from joker-eph July 31, 2020 13:08

googlebot added the cla: yes label Jul 31, 2020

nouiz changed the title ~~[XLA] When LLVM doesn't know a CC that is more recent, doesn't warn about it~~ [XLA] When LLVM doesn't know a CC that is more recent, warn only to developers. Jul 31, 2020

nouiz force-pushed the upstream_master_tf2 branch from 0bcd929 to 98f83bd Compare July 31, 2020 13:15

gbaned self-assigned this Jul 31, 2020

gbaned added the comp:xla XLA label Jul 31, 2020

gbaned added this to Assigned Reviewer in PR Queue via automation Jul 31, 2020

gbaned requested a review from thomasjoerg July 31, 2020 13:16

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Jul 31, 2020

thomasjoerg previously approved these changes Jul 31, 2020

View reviewed changes

google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jul 31, 2020

kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 31, 2020

sanjoy suggested changes Jul 31, 2020

View reviewed changes

PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Jul 31, 2020

nouiz dismissed thomasjoerg’s stale review via db781e9 July 31, 2020 18:35

nouiz force-pushed the upstream_master_tf2 branch from 98f83bd to db781e9 Compare July 31, 2020 18:35

google-ml-butler bot removed the ready to pull PR ready for merge process label Jul 31, 2020

nouiz force-pushed the upstream_master_tf2 branch from db781e9 to 6c5d67a Compare July 31, 2020 18:41

sanjoy suggested changes Jul 31, 2020

View reviewed changes

gbaned added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label Aug 3, 2020

gbaned added stat:awaiting response Status - Awaiting response from author and removed stat:awaiting tensorflower Status - Awaiting response from tensorflower labels Aug 5, 2020

nouiz force-pushed the upstream_master_tf2 branch from 6c5d67a to b05fdf4 Compare August 17, 2020 14:51

tensorflowbutler removed the stat:awaiting response Status - Awaiting response from author label Aug 19, 2020

timshen91 reviewed Aug 19, 2020

View reviewed changes

tensorflow/compiler/xla/service/gpu/llvm_gpu_backend/gpu_backend_lib.cc Outdated Show resolved Hide resolved

sanjoy suggested changes Aug 19, 2020

View reviewed changes

tensorflow/compiler/xla/service/gpu/llvm_gpu_backend/gpu_backend_lib.cc Outdated Show resolved Hide resolved

When LLVM doesn't know a CC that is more recent, doesn't warn about i…

d1abd38

…t. The end user can't do anything about it.

nouiz force-pushed the upstream_master_tf2 branch from b05fdf4 to d1abd38 Compare August 19, 2020 14:28

sanjoy approved these changes Aug 19, 2020

View reviewed changes

PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Aug 19, 2020

google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Aug 19, 2020

kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 19, 2020

tensorflow-copybara merged commit 6792e84 into tensorflow:master Aug 19, 2020

PR Queue automation moved this from Approved by Reviewer to Merged Aug 19, 2020

nouiz deleted the upstream_master_tf2 branch August 20, 2020 17:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[XLA] When LLVM doesn't know a CC that is more recent, warn only to developers. #41936

[XLA] When LLVM doesn't know a CC that is more recent, warn only to developers. #41936

nouiz commented Jul 31, 2020

thomasjoerg Jul 31, 2020

sanjoy Jul 31, 2020

sanjoy Jul 31, 2020

sanjoy Jul 31, 2020

nouiz commented Jul 31, 2020

sanjoy left a comment

sanjoy Jul 31, 2020

nouiz Jul 31, 2020

sanjoy Aug 1, 2020

gbaned commented Aug 3, 2020

trentlo commented Aug 4, 2020

nouiz commented Aug 17, 2020

nouiz commented Aug 19, 2020

sanjoy left a comment

[XLA] When LLVM doesn't know a CC that is more recent, warn only to developers. #41936

[XLA] When LLVM doesn't know a CC that is more recent, warn only to developers. #41936

Conversation

nouiz commented Jul 31, 2020

thomasjoerg Jul 31, 2020

Choose a reason for hiding this comment

sanjoy Jul 31, 2020

Choose a reason for hiding this comment

sanjoy Jul 31, 2020

Choose a reason for hiding this comment

sanjoy Jul 31, 2020

Choose a reason for hiding this comment

nouiz commented Jul 31, 2020

sanjoy left a comment

Choose a reason for hiding this comment

sanjoy Jul 31, 2020

Choose a reason for hiding this comment

nouiz Jul 31, 2020

Choose a reason for hiding this comment

sanjoy Aug 1, 2020

Choose a reason for hiding this comment

gbaned commented Aug 3, 2020

trentlo commented Aug 4, 2020

nouiz commented Aug 17, 2020

nouiz commented Aug 19, 2020

sanjoy left a comment

Choose a reason for hiding this comment