Attachment 356271 Details for Bug 192271 – proposed patch.

[patch] proposed patch.

bug-192271.patch (text/plain), 17.02 KB, created by Mark Lam on 2018-11-30 17:34:44 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2018-11-30 17:34:44 PST

Size: 17.02 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 238767)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2018-11-30  Mark Lam  <mark.lam@apple.com>
>+
>+        Fix the bytecode code generator scripts to pretty print BytecodeStructs.h and BytecodeIndices.h.
>+        https://bugs.webkit.org/show_bug.cgi?id=192271
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This makes the generated code style compliant and human readable.
>+
>+        * generator/Argument.rb:
>+        * generator/DSL.rb:
>+        * generator/Fits.rb:
>+        * generator/Metadata.rb:
>+        * generator/Opcode.rb:
>+
> 2018-11-30  Dean Jackson  <dino@apple.com>
> 
>         Try to fix Windows build by using strcmp instead of strcasecmp.
>Index: Source/JavaScriptCore/generator/Argument.rb
>===================================================================
>--- Source/JavaScriptCore/generator/Argument.rb	(revision 238767)
>+++ Source/JavaScriptCore/generator/Argument.rb	(working copy)
>@@ -60,24 +60,24 @@ class Argument
> 
>     def setter
>         <<-EOF
>-        template<typename Functor>
>-        void set#{capitalized_name}(#{@type.to_s} value, Functor func)
>-        {
>-            if (isWide())
>-                set#{capitalized_name}<OpcodeSize::Wide>(value, func);
>-            else
>-                set#{capitalized_name}<OpcodeSize::Narrow>(value, func);
>-        }
>+    template<typename Functor>
>+    void set#{capitalized_name}(#{@type.to_s} value, Functor func)
>+    {
>+        if (isWide())
>+            set#{capitalized_name}<OpcodeSize::Wide>(value, func);
>+        else
>+            set#{capitalized_name}<OpcodeSize::Narrow>(value, func);
>+    }
> 
>-        template <OpcodeSize size, typename Functor>
>-        void set#{capitalized_name}(#{@type.to_s} value, Functor func)
>-        {
>-            if (!#{Fits::check "size", "value", @type})
>-                value = func();
>-            auto* stream = bitwise_cast<typename TypeBySize<size>::type*>(reinterpret_cast<uint8_t*>(this) + #{@index} * size + PaddingBySize<size>::value);
>-            *stream = #{Fits::convert "size", "value", @type};
>-        }
>-        EOF
>+    template <OpcodeSize size, typename Functor>
>+    void set#{capitalized_name}(#{@type.to_s} value, Functor func)
>+    {
>+        if (!#{Fits::check "size", "value", @type})
>+            value = func();
>+        auto* stream = bitwise_cast<typename TypeBySize<size>::type*>(reinterpret_cast<uint8_t*>(this) + #{@index} * size + PaddingBySize<size>::value);
>+        *stream = #{Fits::convert "size", "value", @type};
>+    }
>+EOF
>     end
> 
>     def capitalized_name
>Index: Source/JavaScriptCore/generator/DSL.rb
>===================================================================
>--- Source/JavaScriptCore/generator/DSL.rb	(revision 238767)
>+++ Source/JavaScriptCore/generator/DSL.rb	(working copy)
>@@ -114,26 +114,24 @@ module DSL
>             opcodes = opcodes_for(:emit_in_structs_file)
> 
>             template.prefix = <<-EOF
>-            #pragma once
>+#pragma once
> 
>-            #include "ArithProfile.h"
>-            #include "BytecodeDumper.h"
>-            #include "BytecodeGenerator.h"
>-            #include "Fits.h"
>-            #include "GetByIdMetadata.h"
>-            #include "Instruction.h"
>-            #include "Opcode.h"
>-            #include "ToThisStatus.h"
>+#include "ArithProfile.h"
>+#include "BytecodeDumper.h"
>+#include "BytecodeGenerator.h"
>+#include "Fits.h"
>+#include "GetByIdMetadata.h"
>+#include "Instruction.h"
>+#include "Opcode.h"
>+#include "ToThisStatus.h"
> 
>-            namespace JSC {
>-            EOF
>+namespace JSC {
>+EOF
> 
>             template.body = <<-EOF
>-            #{opcodes.map(&:struct).join("\n")}
>-
>-            #{Opcode.dump_bytecode(opcodes)}
>-            EOF
>-
>+#{opcodes.map(&:struct).join("\n")}
>+#{Opcode.dump_bytecode(opcodes)}
>+EOF
>             template.suffix = "} // namespace JSC"
>         end
>     end
>@@ -152,9 +150,9 @@ module DSL
>         opcodes = opcodes_for(:emit_in_structs_file)
> 
>         GeneratedFile::create(indices_filename, bytecode_list) do |template|
>-            template.prefix = "namespace JSC {"
>+            template.prefix = "namespace JSC {\n"
>             template.body = opcodes.map(&:struct_indices).join("\n")
>-            template.suffix = "} // namespace JSC"
>+            template.suffix = "\n} // namespace JSC"
>         end
>     end
> 
>Index: Source/JavaScriptCore/generator/Fits.rb
>===================================================================
>--- Source/JavaScriptCore/generator/Fits.rb	(revision 238767)
>+++ Source/JavaScriptCore/generator/Fits.rb	(working copy)
>@@ -31,6 +31,6 @@ module Fits
>     end
> 
>     def self.write(size, name, type)
>-        "__generator->write(#{convert(size, name, type)});"
>+        "gen->write(#{convert(size, name, type)});"
>     end
> end
>Index: Source/JavaScriptCore/generator/Metadata.rb
>===================================================================
>--- Source/JavaScriptCore/generator/Metadata.rb	(revision 238767)
>+++ Source/JavaScriptCore/generator/Metadata.rb	(working copy)
>@@ -42,59 +42,59 @@ class Metadata
>     def struct(op)
>         return if empty?
> 
>-        def convertFields(fields)
>+        def convertFields(prefix, fields)
>             fields.map do |field, type|
>                 if type.kind_of? Hash
>-                    "union {\n#{convertFields(type)}\n};"
>+                    "#{prefix}union {\n#{convertFields(prefix + '    ', type)}\n#{prefix}};"
>                 else
>-                    "#{type.to_s} #{field.to_s};"
>+                    "#{prefix}#{type.to_s} #{field.to_s};"
>                 end
>             end.join("\n")
>         end
> 
>-        fields = convertFields(@fields)
>+        fields = convertFields("        ", @fields)
> 
>         inits = nil
>         if @initializers && (not @initializers.empty?)
>-            inits = ": " + @initializers.map do |metadata, arg|
>+            inits = "\n            : " + @initializers.map do |metadata, arg|
>                 "#{metadata}(__op.#{arg})"
>-            end.join(", ")
>+            end.join("\n            , ").concat("\n       ");
>         end
> 
>         <<-EOF
>-        struct Metadata {
>-            WTF_MAKE_NONCOPYABLE(Metadata);
> 
>-        public:
>-            Metadata(const #{op.capitalized_name}&#{" __op" if inits})
>-            #{inits}
>-            { }
>+    struct Metadata {
>+        WTF_MAKE_NONCOPYABLE(Metadata);
> 
>-            #{fields}
>-        };
>-        EOF
>+    public:
>+        Metadata(const #{op.capitalized_name}&#{" __op" if inits})#{inits} { }
>+
>+#{fields}
>+    };
>+EOF
>     end
> 
>     def accessor
>         return if empty?
> 
>         <<-EOF
>-        Metadata& metadata(CodeBlock* codeBlock) const
>-        {
>-            return codeBlock->metadata<Metadata>(opcodeID, #{Metadata.field_name});
>-        }
>-
>-        Metadata& metadata(ExecState* exec) const
>-        {
>-            return metadata(exec->codeBlock());
>-        }
>-        EOF
>+
>+    Metadata& metadata(CodeBlock* codeBlock) const
>+    {
>+        return codeBlock->metadata<Metadata>(opcodeID, #{Metadata.field_name});
>+    }
>+
>+    Metadata& metadata(ExecState* exec) const
>+    {
>+        return metadata(exec->codeBlock());
>+    }
>+EOF
>     end
> 
>-    def field
>+    def field(prefix)
>         return if empty?
> 
>-        "unsigned #{Metadata.field_name};"
>+        "\n#{prefix}unsigned #{Metadata.field_name};"
>     end
> 
>     def load_from_stream(index, size)
>@@ -106,8 +106,9 @@ class Metadata
>     def create_emitter_local
>         return if empty?
> 
>-        <<-EOF
>-        auto #{emitter_local.name} = __generator->addMetadataFor(opcodeID);
>+        <<-EOF.chomp
>+
>+        auto #{emitter_local.name} = gen->addMetadataFor(opcodeID);
>         EOF
>     end
> 
>Index: Source/JavaScriptCore/generator/Opcode.rb
>===================================================================
>--- Source/JavaScriptCore/generator/Opcode.rb	(revision 238767)
>+++ Source/JavaScriptCore/generator/Opcode.rb	(working copy)
>@@ -56,10 +56,14 @@ class Opcode
> 
>     def print_args(&block)
>         return if @args.nil?
>-
>         @args.map(&block).join "\n"
>     end
> 
>+    def print_members(prefix, &block)
>+        return if @args.nil?
>+        @args.map(&block).map { |arg| "#{prefix}#{arg}" }.join "\n"
>+    end
>+
>     def capitalized_name
>         name.split('_').map(&:capitalize).join
>     end
>@@ -76,33 +80,27 @@ class Opcode
>         @args.map(&:name).unshift("").join(", ")
>     end
> 
>-    def map_fields_with_size(size, &block)
>+    def map_fields_with_size(prefix, size, &block)
>         args = [Argument.new("opcodeID", :unsigned, 0)]
>         args += @args.dup if @args
>         unless @metadata.empty?
>             args << @metadata.emitter_local
>         end
>-        args.map { |arg| block.call(arg, size) }
>+        args.map { |arg| "#{prefix}#{block.call(arg, size)}" }
>     end
> 
>     def struct
>         <<-EOF
>-        struct #{capitalized_name} : public Instruction {
>-            #{opcodeID}
>-
>-            #{emitter}
>-
>-            #{dumper}
>-
>-            #{constructors}
>+struct #{capitalized_name} : public Instruction {
>+    #{opcodeID}
> 
>-            #{setters}
>-
>-            #{metadata_struct_and_accessor}
>-
>-            #{members}
>-        };
>-        EOF
>+#{emitter}
>+#{dumper}
>+#{constructors}
>+#{setters}#{metadata_struct_and_accessor}
>+#{members}
>+};
>+EOF
>     end
> 
>     def opcodeID
>@@ -113,90 +111,92 @@ class Opcode
>         op_wide = Argument.new("op_wide", :unsigned, 0)
>         metadata_param = @metadata.empty? ? "" : ", #{@metadata.emitter_local.create_param}"
>         metadata_arg = @metadata.empty? ? "" : ", #{@metadata.emitter_local.name}"
>-        <<-EOF
>-        static void emit(BytecodeGenerator* __generator#{typed_args})
>-        {
>-            __generator->recordOpcode(opcodeID);
>-            #{@metadata.create_emitter_local}
>-            emit<OpcodeSize::Narrow, NoAssert, false>(__generator#{untyped_args}#{metadata_arg}) || emit<OpcodeSize::Wide, Assert, false>(__generator#{untyped_args}#{metadata_arg});
>-        }
>-
>-        #{%{
>-        template<OpcodeSize size, FitsAssertion shouldAssert = Assert>
>-        static bool emit(BytecodeGenerator* __generator#{typed_args})
>-        {
>-            #{@metadata.create_emitter_local}
>-            return emit<size, shouldAssert>(__generator#{untyped_args}#{metadata_arg});
>-        }
>-        } unless @metadata.empty?}
>-
>-        template<OpcodeSize size, FitsAssertion shouldAssert = Assert, bool recordOpcode = true>
>-        static bool emit(BytecodeGenerator* __generator#{typed_args}#{metadata_param})
>-        {
>-            if (recordOpcode)
>-                __generator->recordOpcode(opcodeID);
>-            bool didEmit = emitImpl<size>(__generator#{untyped_args}#{metadata_arg});
>-            if (shouldAssert == Assert)
>-                ASSERT(didEmit);
>-            return didEmit;
>-        }
>-
>-        private:
>-        template<OpcodeSize size>
>-        static bool emitImpl(BytecodeGenerator* __generator#{typed_args}#{metadata_param})
>-        {
>+        <<-EOF.chomp
>+    static void emit(BytecodeGenerator* gen#{typed_args})
>+    {
>+        gen->recordOpcode(opcodeID);#{@metadata.create_emitter_local}
>+        emit<OpcodeSize::Narrow, NoAssert, false>(gen#{untyped_args}#{metadata_arg})
>+            || emit<OpcodeSize::Wide, Assert, false>(gen#{untyped_args}#{metadata_arg});
>+    }
>+#{%{
>+    template<OpcodeSize size, FitsAssertion shouldAssert = Assert>
>+    static bool emit(BytecodeGenerator* gen#{typed_args})
>+    {#{@metadata.create_emitter_local}
>+        return emit<size, shouldAssert>(gen#{untyped_args}#{metadata_arg});
>+    }
>+} unless @metadata.empty?}
>+    template<OpcodeSize size, FitsAssertion shouldAssert = Assert, bool recordOpcode = true>
>+    static bool emit(BytecodeGenerator* gen#{typed_args}#{metadata_param})
>+    {
>+        if (recordOpcode)
>+            gen->recordOpcode(opcodeID);
>+        bool didEmit = emitImpl<size>(gen#{untyped_args}#{metadata_arg});
>+        if (shouldAssert == Assert)
>+            ASSERT(didEmit);
>+        return didEmit;
>+    }
>+
>+private:
>+    template<OpcodeSize size>
>+    static bool emitImpl(BytecodeGenerator* gen#{typed_args}#{metadata_param})
>+    {
>+        if (size == OpcodeSize::Wide)
>+            gen->alignWideOpcode();
>+        if (#{map_fields_with_size("", "size", &:fits_check).join "\n            && "}
>+            && (size == OpcodeSize::Wide ? #{op_wide.fits_check(Size::Narrow)} : true)) {
>             if (size == OpcodeSize::Wide)
>-                __generator->alignWideOpcode();
>-            if (#{map_fields_with_size("size", &:fits_check).join " && "} && (size == OpcodeSize::Wide ? #{op_wide.fits_check(Size::Narrow)} : true)) {
>-                if (size == OpcodeSize::Wide)
>-                    #{op_wide.fits_write Size::Narrow}
>-                #{map_fields_with_size("size", &:fits_write).join "\n"}
>-                return true;
>-            }
>-            return false;
>+                #{op_wide.fits_write Size::Narrow}
>+#{map_fields_with_size("            ", "size", &:fits_write).join "\n"}
>+            return true;
>         }
>-        public:
>-        EOF
>+        return false;
>+    }
>+
>+public:
>+EOF
>     end
> 
>     def dumper
>         <<-EOF
>-        template<typename Block>
>-        void dump(BytecodeDumper<Block>* __dumper, InstructionStream::Offset __location, bool __isWide)
>-        {
>-            __dumper->printLocationAndOp(__location, &"*#{@name}"[!__isWide]);
>-            #{print_args { |arg|
>-            <<-EOF
>-                __dumper->dumpOperand(#{arg.name}, #{arg.index == 1});
>-            EOF
>-            }}
>-        }
>-        EOF
>+    template<typename Block>
>+    void dump(BytecodeDumper<Block>* dumper, InstructionStream::Offset __location, bool __isWide)
>+    {
>+        dumper->printLocationAndOp(__location, &"*#{@name}"[!__isWide]);
>+#{print_args { |arg|
>+<<-EOF.chomp
>+        dumper->dumpOperand(#{arg.name}, #{arg.index == 1});
>+EOF
>+    }}
>+    }
>+EOF
>     end
> 
>     def constructors
>         fields = (@args || []) + (@metadata.empty? ? [] : [@metadata])
>-        init = ->(size) { fields.empty?  ? "" : ": #{fields.map.with_index { |arg, i| arg.load_from_stream(i + 1, size) }.join ",\n" }" }
>+        init = ->(size) { fields.empty?  ? "" : ": #{fields.map.with_index { |arg, i| arg.load_from_stream(i + 1, size) }.join "\n        , " }" }
> 
>         <<-EOF
>-        #{capitalized_name}(const uint8_t* stream)
>-            #{init.call("OpcodeSize::Narrow")}
>-        { ASSERT_UNUSED(stream, stream[0] == opcodeID); }
>-
>-        #{capitalized_name}(const uint32_t* stream)
>-            #{init.call("OpcodeSize::Wide")}
>-        { ASSERT_UNUSED(stream, stream[0] == opcodeID); }
>-
>-        static #{capitalized_name} decode(const uint8_t* stream)
>-        {
>-            if (*stream != op_wide)
>-                return { stream };
>-
>-            auto wideStream = bitwise_cast<const uint32_t*>(stream + 1);
>-            return { wideStream };
>-        }
>-
>-        EOF
>+    #{capitalized_name}(const uint8_t* stream)
>+        #{init.call("OpcodeSize::Narrow")}
>+    {
>+        ASSERT_UNUSED(stream, stream[0] == opcodeID);
>+    }
>+
>+    #{capitalized_name}(const uint32_t* stream)
>+        #{init.call("OpcodeSize::Wide")}
>+    {
>+        ASSERT_UNUSED(stream, stream[0] == opcodeID);
>+    }
>+
>+    static #{capitalized_name} decode(const uint8_t* stream)
>+    {
>+        if (*stream != op_wide)
>+            return { stream };
>+
>+        auto wideStream = bitwise_cast<const uint32_t*>(stream + 1);
>+        return { wideStream };
>+    }
>+EOF
>     end
> 
>     def setters
>@@ -204,18 +204,15 @@ class Opcode
>     end
> 
>     def metadata_struct_and_accessor
>-        <<-EOF
>-        #{@metadata.struct(self)}
>-
>-        #{@metadata.accessor}
>-        EOF
>+        <<-EOF.chomp
>+#{@metadata.struct(self)}#{@metadata.accessor}
>+EOF
>     end
> 
>     def members
>-        <<-EOF
>-        #{print_args(&:field)}
>-        #{@metadata.field}
>-        EOF
>+        <<-EOF.chomp
>+#{print_members("    ", &:field)}#{@metadata.field("    ")}
>+EOF
>     end
> 
>     def set_entry_address(id)
>@@ -248,22 +245,22 @@ class Opcode
>     end
> 
>     def self.dump_bytecode(opcodes)
>-        <<-EOF
>-        template<typename Block>
>-        static void dumpBytecode(BytecodeDumper<Block>* __dumper, InstructionStream::Offset __location, const Instruction* __instruction)
>-        {
>-            switch (__instruction->opcodeID()) {
>-            #{opcodes.map { |op|
>-                <<-EOF
>-                case #{op.name}:
>-                    __instruction->as<#{op.capitalized_name}>().dump(__dumper, __location, __instruction->isWide());
>-                    break;
>-                EOF
>-            }.join "\n"}
>-            default:
>-                ASSERT_NOT_REACHED();
>-            }
>-        }
>-        EOF
>+        <<-EOF.chomp
>+template<typename Block>
>+static void dumpBytecode(BytecodeDumper<Block>* dumper, InstructionStream::Offset __location, const Instruction* __instruction)
>+{
>+    switch (__instruction->opcodeID()) {
>+#{opcodes.map { |op|
>+        <<-EOF.chomp
>+    case #{op.name}:
>+        __instruction->as<#{op.capitalized_name}>().dump(dumper, __location, __instruction->isWide());
>+        break;
>+EOF
>+    }.join "\n"}
>+    default:
>+        ASSERT_NOT_REACHED();
>+    }
>+}
>+EOF
>     end
> end

Flags:

keith_miller: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 192271: 356271