Minor fixes (#67)

* Minor fixes

- the '-cc1' flag is required for commands extracted from .cmd files- Sometimes it's not in the .cmd file (Not sure how I managed it, but it happened)
- disabled yapf formatting on data_reader's dataset builder
- gracefully handle when a compile_command doesn't have the -o option

Author: Northbadge

check-license.sh

@@ -1,3 +1,4 @@
+#!/bin/bash
 # Copyright 2020 Google LLC
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -12,27 +13,33 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-#!/bin/bash
 
 exitcode=0
-for fname in `find . -name "*.py"`; 
-do
-  diff -q <(head -14 $fname) <(cat license-header.txt) > /dev/null
-  if [ $? -ne 0 ]
+
+check_license () {
+  HEADER=$(head -1 $1)
+  if [[ $HEADER == \#!* ]] # Ignore shebang line
+  then
+    HEADER=$(head -$3 $1 | sed 1d)
+  else
+    HEADER=$(head -$(($3 - 1)) $fname)
+  fi
+  if [[ "$HEADER" != "$2" ]]
   then
-    echo "$fname does not have license header. Please copy and paste ./license-header.txt"
+    echo "$1 does not have license header. Please copy and paste ./license-header.txt"
     exitcode=1
   fi
+}
+
+for fname in `find . -name "*.py"`;
+do
+  check_license $fname "$(cat license-header.txt)" 15
 done
 
-for fname in `find . -name "*.sh"`; 
+for fname in `find . -name "*.sh"`;
 do
-  diff -q <(head -13 $fname) <(tail -13 license-header.txt) > /dev/null
-  if [ $? -ne 0 ]
-  then
-    echo "$fname does not have license header. Please copy and paste ./license-header.txt"
-    exitcode=1
-  fi
+  check_license $fname "$(tail -13 license-header.txt)" 14
 done
 
 exit ${exitcode}
+

compiler_opt/rl/corpus.py

@@ -125,4 +125,8 @@ def _load_and_parse_command(
 
   cmdline.extend(additional_flags)
 
+  # The options read from a .cmd file must be run with -cc1
+  if not cmd_override and cmdline[0] != '-cc1':
+    raise ValueError('-cc1 flag not present in .cmd file.')
+
   return cmdline

compiler_opt/rl/corpus_test.py

@@ -109,6 +109,16 @@ def test_override(self):
             cmd_override=cmd_override),
         ['-fix-all-bugs', '-x', 'ir', 'this!path#cant$exist/hi.bc'])
 
+  def test_cc1_exists(self):
+    data = ['-fix-all-bugs', '-xyz']
+    argfile = self.create_tempfile(content='\0'.join(data), file_path='hi.cmd')
+    module_path = argfile.full_path[:-4]
+    self.assertRaises(
+        ValueError,
+        corpus._load_and_parse_command,
+        module_path=module_path,
+        has_thinlto=False)
+
 
 class ModuleSpecTest(tf.test.TestCase):
 
@@ -125,7 +135,7 @@ def test_get_without_thinlto(self):
     tempdir.create_file('1.bc')
     tempdir.create_file('1.cmd', content='\0'.join(['-cc1']))
     tempdir.create_file('2.bc')
-    tempdir.create_file('2.cmd', content='\0'.join(['-O3']))
+    tempdir.create_file('2.cmd', content='\0'.join(['-cc1', '-O3']))
 
     ms_list = corpus.build_modulespecs_from_datapath(
         tempdir.full_path, additional_flags=('-add',))
@@ -137,8 +147,9 @@ def test_get_without_thinlto(self):
                      ('-cc1', '-x', 'ir', tempdir.full_path + '/1.bc', '-add'))
 
     self.assertEqual(ms2.name, '2')
-    self.assertEqual(ms2.exec_cmd,
-                     ('-O3', '-x', 'ir', tempdir.full_path + '/2.bc', '-add'))
+    self.assertEqual(
+        ms2.exec_cmd,
+        ('-cc1', '-O3', '-x', 'ir', tempdir.full_path + '/2.bc', '-add'))
 
   def test_get_with_thinlto(self):
     corpus_description = {'modules': ['1', '2'], 'has_thinlto': True}
@@ -151,7 +162,8 @@ def test_get_with_thinlto(self):
         '1.cmd', content='\0'.join(['-cc1', '-fthinlto-index=xyz']))
     tempdir.create_file('2.bc')
     tempdir.create_file('2.thinlto.bc')
-    tempdir.create_file('2.cmd', content='\0'.join(['-fthinlto-index=abc']))
+    tempdir.create_file(
+        '2.cmd', content='\0'.join(['-cc1', '-fthinlto-index=abc']))
 
     ms_list = corpus.build_modulespecs_from_datapath(
         tempdir.full_path,
@@ -168,7 +180,7 @@ def test_get_with_thinlto(self):
 
     self.assertEqual(ms2.name, '2')
     self.assertEqual(ms2.exec_cmd,
-                     ('-x', 'ir', tempdir.full_path + '/2.bc',
+                     ('-cc1', '-x', 'ir', tempdir.full_path + '/2.bc',
                       '-fthinlto-index=' + tempdir.full_path + '/2.thinlto.bc',
                       '-mllvm', '-thinlto-assume-merged', '-add'))
 

compiler_opt/rl/data_reader.py

@@ -165,13 +165,19 @@ def create_sequence_example_dataset_fn(
   def _sequence_example_dataset_fn(sequence_examples):
     # Data collector returns empty strings for corner cases, filter them out
     # here.
-    dataset = (
-        tf.data.Dataset.from_tensor_slices(sequence_examples).filter(
-            lambda string: tf.strings.length(string) > 0).map(parser_fn).filter(
-                lambda traj: tf.size(traj.reward) > 2).unbatch().batch(
-                    train_sequence_length, drop_remainder=True).cache().shuffle(
-                        trajectory_shuffle_buffer_size).batch(
-                            batch_size, drop_remainder=True))
+    # yapf: disable - Looks better hand formatted
+    dataset = (tf.data.Dataset
+                .from_tensor_slices(sequence_examples)
+                .filter(lambda string: tf.strings.length(string) > 0)
+                .map(parser_fn)
+                .filter(lambda traj: tf.size(traj.reward) > 2)
+                .unbatch()
+                .batch(train_sequence_length, drop_remainder=True)
+                .cache()
+                .shuffle(trajectory_shuffle_buffer_size)
+                .batch(batch_size, drop_remainder=True)
+               )
+    # yapf: enable
     return dataset
 
   return _sequence_example_dataset_fn

compiler_opt/tools/extract_ir.py

@@ -235,13 +235,18 @@ def extract(self,
         is_thinlto=thinlto_build == 'distributed')
 
 
-def convert_compile_command_to_objectfile(command: Dict[str, str],
-                                          output_dir: str):
+def convert_compile_command_to_objectfile(
+    command: Dict[str, str], output_dir: str) -> Optional[TrainingIRExtractor]:
   obj_base_dir = command['directory']
   cmd = command['command']
 
   cmd_parts = cmd.split()
-  obj_index = cmd_parts.index('-o') + 1
+  try:
+    obj_index = cmd_parts.index('-o') + 1
+  except ValueError:
+    # This could happen if there are non-clang commands in compile_commands.json
+    logging.info('Command has no -o option: %s', cmd)
+    return None
   obj_rel_path = cmd_parts[obj_index]
   # TODO(mtrofin): is the obj_base_dir correct for thinlto index bc files?
   return TrainingIRExtractor(
@@ -252,10 +257,12 @@ def convert_compile_command_to_objectfile(command: Dict[str, str],
 
 def load_from_compile_commands(json_array: List[Dict[str, str]],
                                output_dir: str) -> List[TrainingIRExtractor]:
-  return [
+  objs = [
       convert_compile_command_to_objectfile(cmd, output_dir)
       for cmd in json_array
   ]
+  # Filter out None, in case there were non-clang commands in the .json
+  return [obj for obj in objs if obj is not None]
 
 
 def load_from_lld_params(params_array: List[str], obj_base_dir: str,

compiler_opt/tools/extract_ir_test.py

@@ -34,12 +34,16 @@ def test_one_conversion(self):
             'command': '-cc1 -c /some/path/lib/foo/bar.cc -o lib/bar.o',
             'file': '/some/path/lib/foo/bar.cc'
         }, '/corpus/destination/path')
+    self.assertIsNotNone(obj)
+    # pytype: disable=attribute-error
+    # Pytype complains about obj being None
     self.assertEqual(obj.input_obj(), '/output/directory/lib/bar.o')
     self.assertEqual(obj.relative_output_path(), 'lib/bar.o')
     self.assertEqual(obj.cmd_file(), '/corpus/destination/path/lib/bar.o.cmd')
     self.assertEqual(obj.bc_file(), '/corpus/destination/path/lib/bar.o.bc')
     self.assertEqual(obj.thinlto_index_file(),
                      '/corpus/destination/path/lib/bar.o.thinlto.bc')
+    # pytype: enable=attribute-error
 
   def test_arr_conversion(self):
     res = extract_ir.load_from_compile_commands([{

compiler_opt/tools/generate_default_trace_test.py

@@ -77,7 +77,7 @@ def test_api(self, mock_get_runner):
 
       with tf.io.gfile.GFile(
           os.path.join(tmp_dir.full_path, module_name + '.cmd'), 'w') as f:
-        f.write(module_name)
+        f.write('-cc1')
 
     mock_compilation_runner = MockCompilationRunner()
     mock_get_runner.return_value = mock_compilation_runner