[Webkit-unassigned] [Bug 130523] Need target to produce llvm ir

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=130523


Mark Rowe (bdash) <mrowe at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #228916|review?, commit-queue?      |review+, commit-queue-
               Flag|                            |




--- Comment #29 from Mark Rowe (bdash) <mrowe at apple.com>  2014-04-09 16:47:28 PST ---
(From update of attachment 228916)
View in context: https://bugs.webkit.org/attachment.cgi?id=228916&action=review

r=me with a few comments. If you upload a version that addresses those we can get it cq+d.

> Source/JavaScriptCore/ChangeLog:12
> +        * build_symbol_table_index.py: Added.
> +        * build_symbol_table_index.sh: Added.
> +        * Configurations/CompileRuntimeToLLVMIR.xcconfig: Added.
> +        * copy_llvm_ir_to_derived_sources.sh: Added.

All of the other scripts at the top level of JavaScriptCore are named with lower-case-and-hyphens rather than using underscores.

> Source/JavaScriptCore/build_symbol_table_index.py:20
> +    print ("Failed to build index table at " + binary_file_directory)

You've an unnecessary space after print.

> Source/JavaScriptCore/build_symbol_table_index.py:38
> +    bitcode_basename = os.path.basename(bitcode_file)
> +    binary_file = os.path.join(binary_file_directory, bitcode_basename[:-2] + "o")

Why do we need to look at the version of the file with a .o suffix? Is that because the .bc file has the wrong modification time due to being copied? Can we switch to using "cp -p" to preserve the modification time on the .bc file, then ignore the .o files in this script?

> Source/JavaScriptCore/build_symbol_table_index.sh:4
> +OBJ_DIR=${BUILT_PRODUCTS_DIR}/DerivedSources/JavaScriptCoreRuntime
> +SAVE_DIR=${BUILT_PRODUCTS_DIR}/${JAVASCRIPTCORE_RESOURCES_DIR}/Runtime

These variable names don't match what the directories are. The former is the derived sources directory, not an object directory, and the latter is the installed location.

> Source/JavaScriptCore/copy_llvm_ir_to_derived_sources.sh:4
> +OBJ_DIR=${TARGET_TEMP_DIR}/Objects-${CURRENT_VARIANT}
> +SAVE_DIR=${BUILT_PRODUCTS_DIR}/DerivedSources/JavaScriptCoreRuntime

I'd suggest renaming SAVE_DIR to match whatever you change OBJ_DIR to in the other script.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.